diff options
210 files changed, 2158 insertions, 791 deletions
diff --git a/.codeclimate.yml b/.codeclimate.yml index 1f0f067c5b..d59a0780d1 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -1,3 +1,25 @@ +checks: + argument-count: + enabled: false + complex-logic: + enabled: false + file-lines: + enabled: false + method-complexity: + enabled: false + method-count: + enabled: false + method-lines: + enabled: false + nested-control-flow: + enabled: false + return-statements: + enabled: false + similar-code: + enabled: false + identical-code: + enabled: false + engines: rubocop: enabled: true diff --git a/.rubocop.yml b/.rubocop.yml index f6259fe432..a04de4b497 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -58,6 +58,9 @@ Layout/IndentationConsistency: Layout/IndentationWidth: Enabled: true +Layout/LeadingCommentSpace: + Enabled: true + Layout/SpaceAfterColon: Enabled: true @@ -73,6 +76,9 @@ Layout/SpaceAroundKeyword: Layout/SpaceAroundOperators: Enabled: true +Layout/SpaceBeforeComma: + Enabled: true + Layout/SpaceBeforeFirstArg: Enabled: true diff --git a/.travis.yml b/.travis.yml index 290e0b5f2b..be41734a57 100644 --- a/.travis.yml +++ b/.travis.yml @@ -106,17 +106,17 @@ matrix: - "GEM=ar:postgresql POSTGRES=9.2" addons: postgresql: "9.2" - - rvm: jruby-9.1.14.0 + - rvm: jruby-9.1.15.0 jdk: oraclejdk8 env: - "GEM=ap" - - rvm: jruby-9.1.14.0 + - rvm: jruby-9.1.15.0 jdk: oraclejdk8 env: - "GEM=am,amo,aj" allow_failures: - rvm: ruby-head - - rvm: jruby-9.1.14.0 + - rvm: jruby-9.1.15.0 - env: "GEM=ac:integration" fast_finish: true @@ -58,7 +58,7 @@ gem "bootsnap", ">= 1.1.0", require: false # Active Job. group :job do gem "resque", require: false - gem "resque-scheduler", github: "resque/resque-scheduler", require: false + gem "resque-scheduler", require: false gem "sidekiq", require: false gem "sucker_punch", require: false gem "delayed_job", require: false @@ -66,7 +66,7 @@ group :job do gem "sneakers", require: false gem "que", require: false gem "backburner", require: false - #TODO: add qu after it support Rails 5.1 + # TODO: add qu after it support Rails 5.1 # gem 'qu-rails', github: "bkeepers/qu", branch: "master", require: false # gem "qu-redis", require: false gem "delayed_job_active_record", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 1453bddb34..7bb207027c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -24,16 +24,6 @@ GIT websocket GIT - remote: https://github.com/resque/resque-scheduler.git - revision: 284b862dd63967da1cf3a7e6abd9d2052067c8be - specs: - resque-scheduler (4.3.0) - mono_logger (~> 1.0) - redis (>= 3.3, < 5) - resque (~> 1.26) - rufus-scheduler (~> 3.2) - -GIT remote: https://github.com/robin850/sdoc.git revision: 0e340352f3ab2f196c8a8743f83c2ee286e4f71c branch: upgrade @@ -393,6 +383,11 @@ GEM redis-namespace (~> 1.3) sinatra (>= 0.9.2) vegas (~> 0.1.2) + resque-scheduler (4.3.1) + mono_logger (~> 1.0) + redis (>= 3.3, < 5) + resque (~> 1.26) + rufus-scheduler (~> 3.2) retriable (3.1.1) rubocop (0.51.0) parallel (~> 1.10) @@ -543,7 +538,7 @@ DEPENDENCIES redis (~> 4.0) redis-namespace resque - resque-scheduler! + resque-scheduler rubocop (>= 0.47) sass-rails sdoc! diff --git a/actionmailer/test/message_delivery_test.rb b/actionmailer/test/message_delivery_test.rb index 03e8d4fb66..f8dcb3f4ba 100644 --- a/actionmailer/test/message_delivery_test.rb +++ b/actionmailer/test/message_delivery_test.rb @@ -39,7 +39,7 @@ class MessageDeliveryTest < ActiveSupport::TestCase end test "its message should be a Mail::Message" do - assert_equal Mail::Message , @mail.message.class + assert_equal Mail::Message, @mail.message.class end test "should respond to .deliver_later" do diff --git a/actionmailer/test/url_test.rb b/actionmailer/test/url_test.rb index 82035689ad..3c940bc969 100644 --- a/actionmailer/test/url_test.rb +++ b/actionmailer/test/url_test.rb @@ -105,7 +105,7 @@ class ActionMailerUrlTest < ActionMailer::TestCase assert_url_for "/dummy_model", DummyModel # array - assert_url_for "/dummy_model" , [DummyModel] + assert_url_for "/dummy_model", [DummyModel] end def test_signed_up_with_url diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index c8fb34ed52..384546d7b4 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,29 @@ +* Changed the system tests to set Puma as default server only when the + user haven't specified manually another server. + + *Guillermo Iguaran* + +* Add secure `X-Download-Options` and `X-Permitted-Cross-Domain-Policies` to + default headers set. + + *Guillermo Iguaran* + +* Add headless firefox support to System Tests. + + *bogdanvlviv* + +* Changed the default system test screenshot output from `inline` to `simple`. + + `inline` works well for iTerm2 but not everyone uses iTerm2. Some terminals like + Terminal.app ignore the `inline` and output the path to the file since it can't + render the image. Other terminals, like those on Ubuntu, cannot handle the image + inline, but also don't handle it gracefully and instead of outputting the file + path, it dumps binary into the terminal. + + Commit 9d6e28 fixes this by changing the default for screenshot to be `simple`. + + *Eileen M. Uchitelle* + * Register most popular audio/video/font mime types supported by modern browsers. *Guillermo Iguaran* diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 04fadc90e2..767eddb361 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -415,11 +415,21 @@ module ActionController #:nodoc: allow_forgery_protection end + NULL_ORIGIN_MESSAGE = <<-MSG.strip_heredoc + The browser returned a 'null' origin for a request with origin-based forgery protection turned on. This usually + means you have the 'no-referrer' Referrer-Policy header enabled, or that you the request came from a site that + refused to give its origin. This makes it impossible for Rails to verify the source of the requests. Likely the + best solution is to change your referrer policy to something less strict like same-origin or strict-same-origin. + If you cannot change the referrer policy, you can disable origin checking with the + Rails.application.config.action_controller.forgery_protection_origin_check setting. + MSG + # Checks if the request originated from the same origin by looking at the # Origin header. def valid_request_origin? # :doc: if forgery_protection_origin_check # We accept blank origin headers because some user agents don't send it. + raise InvalidAuthenticityToken, NULL_ORIGIN_MESSAGE if request.origin == "null" request.origin.nil? || request.origin == request.base_url else true diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index ef7c4c4c16..a56ac749f8 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -335,7 +335,7 @@ module ActionController # the same way as <tt>Hash#each_pair</tt>. def each_pair(&block) @parameters.each_pair do |key, value| - yield key, convert_hashes_to_parameters(key, value) + yield [key, convert_hashes_to_parameters(key, value)] end end alias_method :each, :each_pair diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index d10d4faf3d..4883e23d24 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/object/deep_dup" + module ActionDispatch #:nodoc: class ContentSecurityPolicy class Middleware @@ -110,7 +112,7 @@ module ActionDispatch #:nodoc: end def initialize_copy(other) - @directives = copy_directives(other.directives) + @directives = other.directives.deep_dup end DIRECTIVES.each do |name, directive| @@ -174,10 +176,6 @@ module ActionDispatch #:nodoc: end private - def copy_directives(directives) - directives.transform_values { |sources| sources.map(&:dup) } - end - def apply_mappings(sources) sources.map do |source| case source diff --git a/actionpack/lib/action_dispatch/journey/nfa/dot.rb b/actionpack/lib/action_dispatch/journey/nfa/dot.rb index bdb78d8d48..56e9e3c83d 100644 --- a/actionpack/lib/action_dispatch/journey/nfa/dot.rb +++ b/actionpack/lib/action_dispatch/journey/nfa/dot.rb @@ -9,16 +9,16 @@ module ActionDispatch " #{from} -> #{to} [label=\"#{sym || 'ε'}\"];" } - #memo_nodes = memos.values.flatten.map { |n| - # label = n - # if Journey::Route === n - # label = "#{n.verb.source} #{n.path.spec}" - # end - # " #{n.object_id} [label=\"#{label}\", shape=box];" - #} - #memo_edges = memos.flat_map { |k, memos| - # (memos || []).map { |v| " #{k} -> #{v.object_id};" } - #}.uniq + # memo_nodes = memos.values.flatten.map { |n| + # label = n + # if Journey::Route === n + # label = "#{n.verb.source} #{n.path.spec}" + # end + # " #{n.object_id} [label=\"#{label}\", shape=box];" + # } + # memo_edges = memos.flat_map { |k, memos| + # (memos || []).map { |v| " #{k} -> #{v.object_id};" } + # }.uniq <<-eodot digraph nfa { diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 855f2ffa47..95e99987a0 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -26,7 +26,9 @@ module ActionDispatch config.action_dispatch.default_headers = { "X-Frame-Options" => "SAMEORIGIN", "X-XSS-Protection" => "1; mode=block", - "X-Content-Type-Options" => "nosniff" + "X-Content-Type-Options" => "nosniff", + "X-Download-Options" => "noopen", + "X-Permitted-Cross-Domain-Policies" => "none" } config.action_dispatch.cookies_rotations = ActiveSupport::Messages::RotationConfiguration.new diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb index 7246e01cff..99d0c06751 100644 --- a/actionpack/lib/action_dispatch/system_test_case.rb +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -121,11 +121,15 @@ module ActionDispatch # # driven_by :poltergeist # - # driven_by :selenium, using: :firefox + # driven_by :selenium, screen_size: [800, 800] + # + # driven_by :selenium, using: :chrome # # driven_by :selenium, using: :headless_chrome # - # driven_by :selenium, screen_size: [800, 800] + # driven_by :selenium, using: :firefox + # + # driven_by :selenium, using: :headless_firefox def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400], options: {}) self.driver = SystemTesting::Driver.new(driver, using: using, screen_size: screen_size, options: options) end diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb index 2687772b4b..280989a146 100644 --- a/actionpack/lib/action_dispatch/system_testing/driver.rb +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -38,13 +38,24 @@ module ActionDispatch browser_options.args << "--disable-gpu" @options.merge(options: browser_options) + elsif @browser == :headless_firefox + browser_options = Selenium::WebDriver::Firefox::Options.new + browser_options.args << "-headless" + + @options.merge(options: browser_options) else @options end end def browser - @browser == :headless_chrome ? :chrome : @browser + if @browser == :headless_chrome + :chrome + elsif @browser == :headless_firefox + :firefox + else + @browser + end end def register_selenium(app) diff --git a/actionpack/lib/action_dispatch/system_testing/server.rb b/actionpack/lib/action_dispatch/system_testing/server.rb index 8f1b6725b1..4fc1f33767 100644 --- a/actionpack/lib/action_dispatch/system_testing/server.rb +++ b/actionpack/lib/action_dispatch/system_testing/server.rb @@ -20,7 +20,7 @@ module ActionDispatch end def set_server - Capybara.server = :puma, { Silent: self.class.silence_puma } + Capybara.server = :puma, { Silent: self.class.silence_puma } if Capybara.server == Capybara.servers[:default] end def set_port diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 5262e85a28..55ad9c245e 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -453,3 +453,7 @@ end class DrivenBySeleniumWithHeadlessChrome < ActionDispatch::SystemTestCase driven_by :selenium, using: :headless_chrome end + +class DrivenBySeleniumWithHeadlessFirefox < ActionDispatch::SystemTestCase + driven_by :selenium, using: :headless_firefox +end diff --git a/actionpack/test/controller/metal_test.rb b/actionpack/test/controller/metal_test.rb index c235c9df86..c3ebcb22b8 100644 --- a/actionpack/test/controller/metal_test.rb +++ b/actionpack/test/controller/metal_test.rb @@ -9,7 +9,7 @@ class MetalControllerInstanceTests < ActiveSupport::TestCase end end - def test_response_has_default_headers + def test_response_does_not_have_default_headers original_default_headers = ActionDispatch::Response.default_headers ActionDispatch::Response.default_headers = { diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index 43cabae7d2..154430d4b0 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -51,6 +51,14 @@ class ParametersAccessorsTest < ActiveSupport::TestCase @params.each { |key, value| assert_not(value.permitted?) if key == "person" } end + test "each returns key,value array for block with arity 1" do + @params.each do |arg| + assert_kind_of Array, arg + assert_equal "person", arg[0] + assert_kind_of ActionController::Parameters, arg[1] + end + end + test "each_pair carries permitted status" do @params.permit! @params.each_pair { |key, value| assert(value.permitted?) if key == "person" } @@ -60,6 +68,14 @@ class ParametersAccessorsTest < ActiveSupport::TestCase @params.each_pair { |key, value| assert_not(value.permitted?) if key == "person" } end + test "each_pair returns key,value array for block with arity 1" do + @params.each_pair do |arg| + assert_kind_of Array, arg + assert_equal "person", arg[0] + assert_kind_of ActionController::Parameters, arg[1] + end + end + test "empty? returns true when params contains no key/value pairs" do params = ActionController::Parameters.new assert params.empty? diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index eb3d2f34a8..4822d85bcb 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -446,6 +446,19 @@ module RequestForgeryProtectionTests end end + def test_should_raise_for_post_with_null_origin + forgery_protection_origin_check do + session[:_csrf_token] = @token + @controller.stub :form_authenticity_token, @token do + exception = assert_raises(ActionController::InvalidAuthenticityToken) do + @request.set_header "HTTP_ORIGIN", "null" + post :index, params: { custom_authenticity_token: @token } + end + assert_match "The browser returned a 'null' origin for a request", exception.message + end + end + end + def test_should_block_post_with_origin_checking_and_wrong_origin old_logger = ActionController::Base.logger logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 3d98237003..30bea64c55 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -307,7 +307,7 @@ class ResourcesTest < ActionController::TestCase set.draw do resources :messages do member do - match :mark , via: method + match :mark, via: method match :unmark, via: method end end diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index f09051b306..ec939e946a 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -213,7 +213,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal expected, ActiveSupport::JSON.decode(get(u)) end - def test_regexp_precidence + def test_regexp_precedence rs.draw do get "/whois/:domain", constraints: { domain: /\w+\.[\w\.]+/ }, @@ -1687,7 +1687,7 @@ class RouteSetTest < ActiveSupport::TestCase def test_routes_with_symbols set.draw do get "unnamed", controller: :pages, action: :show, name: :as_symbol - get "named" , controller: :pages, action: :show, name: :as_symbol, as: :named + get "named", controller: :pages, action: :show, name: :as_symbol, as: :named end assert_equal({ controller: "pages", action: "show", name: :as_symbol }, set.recognize_path("/unnamed")) assert_equal({ controller: "pages", action: "show", name: :as_symbol }, set.recognize_path("/named")) @@ -1893,7 +1893,7 @@ class RouteSetTest < ActiveSupport::TestCase assert_equal({ controller: "blog", action: "show_date", year: "2006", month: "07", day: "28" }, controller.request.path_parameters) assert_equal("/blog/2006/07/25", controller.url_for(day: 25, only_path: true)) assert_equal("/blog/2005", controller.url_for(year: 2005, only_path: true)) - assert_equal("/blog/show/123", controller.url_for(action: "show" , id: 123, only_path: true)) + assert_equal("/blog/show/123", controller.url_for(action: "show", id: 123, only_path: true)) assert_equal("/blog/2006", controller.url_for(year: 2006, only_path: true)) assert_equal("/blog/2006", controller.url_for(year: 2006, month: nil, only_path: true)) end diff --git a/actionpack/test/controller/url_for_integration_test.rb b/actionpack/test/controller/url_for_integration_test.rb index a7c7356921..a1521da702 100644 --- a/actionpack/test/controller/url_for_integration_test.rb +++ b/actionpack/test/controller/url_for_integration_test.rb @@ -35,7 +35,6 @@ module ActionPack as: "blog" resources :people - #match 'legacy/people' => "people#index", :legacy => "true" get "symbols", controller: :symbols, action: :show, name: :as_symbol get "id_default(/:id)" => "foo#id_default", :id => 1 diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb index 0f79c83b6d..ca83b850d5 100644 --- a/actionpack/test/controller/url_rewriter_test.rb +++ b/actionpack/test/controller/url_rewriter_test.rb @@ -19,7 +19,7 @@ class UrlRewriterTests < ActionController::TestCase def setup @params = {} - @rewriter = Rewriter.new(@request) #.new(@request, @params) + @rewriter = Rewriter.new(@request) @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| r.draw do ActiveSupport::Deprecation.silence do diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb index 8a1ac066e8..7c4a65a633 100644 --- a/actionpack/test/dispatch/content_security_policy_test.rb +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -14,6 +14,15 @@ class ContentSecurityPolicyTest < ActiveSupport::TestCase assert_equal "script-src 'self';", @policy.build end + def test_dup + @policy.img_src :self + @policy.block_all_mixed_content + @policy.upgrade_insecure_requests + @policy.sandbox + copied = @policy.dup + assert_equal copied.build, @policy.build + end + def test_mappings @policy.script_src :data assert_equal "script-src data:;", @policy.build diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index c4ee3add2a..4e350162c9 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -311,13 +311,15 @@ class ResponseTest < ActiveSupport::TestCase end end - test "read x_frame_options, x_content_type_options and x_xss_protection" do + test "read x_frame_options, x_content_type_options, x_xss_protection, x_download_options and x_permitted_cross_domain_policies" do original_default_headers = ActionDispatch::Response.default_headers begin ActionDispatch::Response.default_headers = { "X-Frame-Options" => "DENY", "X-Content-Type-Options" => "nosniff", - "X-XSS-Protection" => "1;" + "X-XSS-Protection" => "1;", + "X-Download-Options" => "noopen", + "X-Permitted-Cross-Domain-Policies" => "none" } resp = ActionDispatch::Response.create.tap { |response| response.body = "Hello" @@ -327,6 +329,8 @@ class ResponseTest < ActiveSupport::TestCase assert_equal("DENY", resp.headers["X-Frame-Options"]) assert_equal("nosniff", resp.headers["X-Content-Type-Options"]) assert_equal("1;", resp.headers["X-XSS-Protection"]) + assert_equal("noopen", resp.headers["X-Download-Options"]) + assert_equal("none", resp.headers["X-Permitted-Cross-Domain-Policies"]) ensure ActionDispatch::Response.default_headers = original_default_headers end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index b2d2bf0416..8f4e7c96a9 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4225,7 +4225,7 @@ class TestGlobRoutingMapper < ActionDispatch::IntegrationTest end end - #include Routes.url_helpers + # include Routes.url_helpers APP = build_app Routes def app; APP end diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 2c43a8c29f..e34426a471 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -8,11 +8,14 @@ require "active_support/messages/rotation_configuration" class CookieStoreTest < ActionDispatch::IntegrationTest SessionKey = "_myapp_session" SessionSecret = "b3c631c314c0bbca50c1b2843150fe33" - Generator = ActiveSupport::LegacyKeyGenerator.new(SessionSecret) + SessionSalt = "authenticated encrypted cookie" + + Generator = ActiveSupport::KeyGenerator.new(SessionSecret, iterations: 1000) Rotations = ActiveSupport::Messages::RotationConfiguration.new - Verifier = ActiveSupport::MessageVerifier.new(SessionSecret, digest: "SHA1") - SignedBar = Verifier.generate(foo: "bar", session_id: SecureRandom.hex(16)) + Encryptor = ActiveSupport::MessageEncryptor.new( + Generator.generate_key(SessionSalt, 32), cipher: "aes-256-gcm", serializer: Marshal + ) class TestController < ActionController::Base def no_session_access @@ -25,12 +28,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def set_session_value session[:foo] = "bar" - render plain: Rack::Utils.escape(Verifier.generate(session.to_hash)) - end - - def set_session_value_expires_in_five_hours - session[:foo] = "bar" - render plain: Rack::Utils.escape(Verifier.generate(session.to_hash, expires_in: 5.hours)) + render body: nil end def get_session_value @@ -72,19 +70,35 @@ class CookieStoreTest < ActionDispatch::IntegrationTest end end + def parse_cookie_from_header + cookie_matches = headers["Set-Cookie"].match(/#{SessionKey}=([^;]+)/) + cookie_matches && cookie_matches[1] + end + + def assert_session_cookie(cookie_string, contents) + assert_includes headers["Set-Cookie"], cookie_string + + session_value = parse_cookie_from_header + session_data = Encryptor.decrypt_and_verify(Rack::Utils.unescape(session_value)) rescue nil + + assert_not_nil session_data, "session failed to decrypt" + assert_equal session_data.slice(*contents.keys), contents + end + def test_setting_session_value with_test_route_set do get "/set_session_value" + assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" end end def test_getting_session_value with_test_route_set do - cookies[SessionKey] = SignedBar + get "/set_session_value" get "/get_session_value" + assert_response :success assert_equal 'foo: "bar"', response.body end @@ -92,8 +106,9 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_getting_session_id with_test_route_set do - cookies[SessionKey] = SignedBar + get "/set_session_value" get "/persistent_session_id" + assert_response :success assert_equal 32, response.body.size session_id = response.body @@ -106,8 +121,12 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_disregards_tampered_sessions with_test_route_set do - cookies[SessionKey] = "BAh7BjoIZm9vIghiYXI%3D--123456780" + encryptor = ActiveSupport::MessageEncryptor.new("A" * 32, cipher: "aes-256-gcm", serializer: Marshal) + + cookies[SessionKey] = encryptor.encrypt_and_sign("foo" => "bar", "session_id" => "abc") + get "/get_session_value" + assert_response :success assert_equal "foo: nil", response.body end @@ -135,19 +154,19 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_does_set_secure_cookies_over_https with_test_route_set(secure: true) do get "/set_session_value", headers: { "HTTPS" => "on" } + assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; secure; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; secure; HttpOnly", "foo" => "bar" end end # {:foo=>#<SessionAutoloadTest::Foo bar:"baz">, :session_id=>"ce8b0752a6ab7c7af3cdb8a80e6b9e46"} - SignedSerializedCookie = "BAh7BzoIZm9vbzodU2Vzc2lvbkF1dG9sb2FkVGVzdDo6Rm9vBjoJQGJhciIIYmF6Og9zZXNzaW9uX2lkIiVjZThiMDc1MmE2YWI3YzdhZjNjZGI4YTgwZTZiOWU0Ng==--2bf3af1ae8bd4e52b9ac2099258ace0c380e601c" + EncryptedSerializedCookie = "9RZ2Fij0qLveUwM4s+CCjGqhpjyUC8jiBIf/AiBr9M3TB8xh2vQZtvSOMfN3uf6oYbbpIDHAcOFIEl69FcW1ozQYeSrCLonYCazoh34ZdYskIQfGwCiSYleVXG1OD9Z4jFqeVArw4Ewm0paOOPLbN1rc6A==--I359v/KWdZ1ok0ey--JFFhuPOY7WUo6tB/eP05Aw==" def test_deserializes_unloaded_classes_on_get_id with_test_route_set do with_autoload_path "session_autoload_test" do - cookies[SessionKey] = SignedSerializedCookie + cookies[SessionKey] = EncryptedSerializedCookie get "/get_session_id" assert_response :success assert_equal "id: ce8b0752a6ab7c7af3cdb8a80e6b9e46", response.body, "should auto-load unloaded class" @@ -158,7 +177,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_deserializes_unloaded_classes_on_get_value with_test_route_set do with_autoload_path "session_autoload_test" do - cookies[SessionKey] = SignedSerializedCookie + cookies[SessionKey] = EncryptedSerializedCookie get "/get_session_value" assert_response :success assert_equal 'foo: #<SessionAutoloadTest::Foo bar:"baz">', response.body, "should auto-load unloaded class" @@ -197,8 +216,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest get "/set_session_value" assert_response :success session_payload = response.body - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" get "/call_reset_session" assert_response :success @@ -216,8 +234,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest with_test_route_set do get "/set_session_value" assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" get "/get_class_after_reset_session" assert_response :success @@ -239,8 +256,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest with_test_route_set do get "/set_session_value" assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" get "/call_session_clear" assert_response :success @@ -253,7 +269,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_persistent_session_id with_test_route_set do - cookies[SessionKey] = SignedBar + get "/set_session_value" get "/persistent_session_id" assert_response :success assert_equal 32, response.body.size @@ -268,8 +284,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_setting_session_id_to_nil_is_respected with_test_route_set do - cookies[SessionKey] = SignedBar - + get "/set_session_value" get "/get_session_id" sid = response.body assert_equal 36, sid.size @@ -283,31 +298,53 @@ class CookieStoreTest < ActionDispatch::IntegrationTest with_test_route_set(expire_after: 5.hours) do # First request accesses the session time = Time.local(2008, 4, 24) + Time.stub :now, time do expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") - cookies[SessionKey] = SignedBar + get "/set_session_value" - get "/set_session_value_expires_in_five_hours" assert_response :success - - cookie_body = response.body - assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; expires=#{expected_expiry}; HttpOnly", "foo" => "bar" end # Second request does not access the session - time = Time.local(2008, 4, 25) + time = time + 3.hours Time.stub :now, time do expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") - cookies[SessionKey] = SignedBar - get "/no_session_access" + assert_response :success + assert_session_cookie "path=/; expires=#{expected_expiry}; HttpOnly", "foo" => "bar" + end + end + end + + def test_session_store_with_expire_after_does_not_accept_expired_session + with_test_route_set(expire_after: 5.hours) do + # First request accesses the session + time = Time.local(2017, 11, 12) + + Time.stub :now, time do + expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") - assert_equal "_myapp_session=#{cookies[SessionKey]}; path=/; expires=#{expected_expiry}; HttpOnly", - headers["Set-Cookie"] + get "/set_session_value" + get "/get_session_value" + + assert_response :success + assert_equal 'foo: "bar"', response.body + assert_session_cookie "path=/; expires=#{expected_expiry}; HttpOnly", "foo" => "bar" + end + + # Second request is beyond the expiry time and the session is invalidated + time += 5.hours + 1.minute + + Time.stub :now, time do + get "/get_session_value" + + assert_response :success + assert_equal "foo: nil", response.body end end end @@ -347,8 +384,14 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def get(path, *args) args[0] ||= {} args[0][:headers] ||= {} - args[0][:headers]["action_dispatch.key_generator"] ||= Generator - args[0][:headers]["action_dispatch.cookies_rotations"] ||= Rotations + args[0][:headers].tap do |config| + config["action_dispatch.secret_key_base"] = SessionSecret + config["action_dispatch.authenticated_encrypted_cookie_salt"] = SessionSalt + config["action_dispatch.use_authenticated_cookie_encryption"] = true + + config["action_dispatch.key_generator"] ||= Generator + config["action_dispatch.cookies_rotations"] ||= Rotations + end super(path, *args) end diff --git a/actionpack/test/dispatch/system_testing/driver_test.rb b/actionpack/test/dispatch/system_testing/driver_test.rb index 75feae6fe0..fcdaf7fb4c 100644 --- a/actionpack/test/dispatch/system_testing/driver_test.rb +++ b/actionpack/test/dispatch/system_testing/driver_test.rb @@ -25,6 +25,14 @@ class DriverTest < ActiveSupport::TestCase assert_equal ({ url: "http://example.com/wd/hub" }), driver.instance_variable_get(:@options) end + test "initializing the driver with a headless firefox" do + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :headless_firefox, screen_size: [1400, 1400], options: { url: "http://example.com/wd/hub" }) + assert_equal :selenium, driver.instance_variable_get(:@name) + assert_equal :headless_firefox, driver.instance_variable_get(:@browser) + assert_equal [1400, 1400], driver.instance_variable_get(:@screen_size) + assert_equal ({ url: "http://example.com/wd/hub" }), driver.instance_variable_get(:@options) + end + test "initializing the driver with a poltergeist" do driver = ActionDispatch::SystemTesting::Driver.new(:poltergeist, screen_size: [1400, 1400], options: { js_errors: false }) assert_equal :poltergeist, driver.instance_variable_get(:@name) diff --git a/actionpack/test/dispatch/system_testing/server_test.rb b/actionpack/test/dispatch/system_testing/server_test.rb index 1866225fc1..95e411faf4 100644 --- a/actionpack/test/dispatch/system_testing/server_test.rb +++ b/actionpack/test/dispatch/system_testing/server_test.rb @@ -6,10 +6,27 @@ require "action_dispatch/system_testing/server" class ServerTest < ActiveSupport::TestCase setup do - ActionDispatch::SystemTesting::Server.new.run + @old_capybara_server = Capybara.server end test "port is always included" do + ActionDispatch::SystemTesting::Server.new.run assert Capybara.always_include_port, "expected Capybara.always_include_port to be true" end + + test "server is changed from `default` to `puma`" do + Capybara.server = :default + ActionDispatch::SystemTesting::Server.new.run + refute_equal Capybara.server, Capybara.servers[:default] + end + + test "server is not changed to `puma` when is different than default" do + Capybara.server = :webrick + ActionDispatch::SystemTesting::Server.new.run + assert_equal Capybara.server, Capybara.servers[:webrick] + end + + teardown do + Capybara.server = @old_capybara_server + end end diff --git a/actionpack/test/dispatch/system_testing/system_test_case_test.rb b/actionpack/test/dispatch/system_testing/system_test_case_test.rb index c6a6aef92b..b078a5abc5 100644 --- a/actionpack/test/dispatch/system_testing/system_test_case_test.rb +++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb @@ -28,6 +28,12 @@ class SetDriverToSeleniumHeadlessChromeTest < DrivenBySeleniumWithHeadlessChrome end end +class SetDriverToSeleniumHeadlessFirefoxTest < DrivenBySeleniumWithHeadlessFirefox + test "uses selenium headless firefox" do + assert_equal :selenium, Capybara.current_driver + end +end + class SetHostTest < DrivenByRackTest test "sets default host" do assert_equal "http://127.0.0.1", Capybara.app_host diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index 4673d7cc11..24c7135c7e 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -18,7 +18,7 @@ module ActionDispatch def test_filename_is_different_object file_str = "foo" uf = Http::UploadedFile.new(filename: file_str, tempfile: Object.new) - assert_not_equal file_str.object_id , uf.original_filename.object_id + assert_not_equal file_str.object_id, uf.original_filename.object_id end def test_filename_should_be_in_utf_8 diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 29cc74471d..183f421bcf 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -30,7 +30,7 @@ module ActionDispatch def test_unicode get "/ほげ", to: "foo#bar" - #match the escaped version of /ほげ + # match the escaped version of /ほげ env = rails_env "PATH_INFO" => "/%E3%81%BB%E3%81%92" called = false router.recognize(env) do |r, params| diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 566e30993b..f42ada0baa 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,7 +1,7 @@ * Add `preload_link_tag` helper This helper that allows to the browser to initiate early fetch of resources - (different to the specified in javascript_include_tag and stylesheet_link_tag). + (different to the specified in `javascript_include_tag` and `stylesheet_link_tag`). Additionally, this sends Early Hints if supported by browser. *Guillermo Iguaran* diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index 02335c72ec..889562c478 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -139,6 +139,11 @@ module ActionView # link_to "Profiles", controller: "profiles" # # => <a href="/profiles">Profiles</a> # + # When name is +nil+ the href is presented instead + # + # link_to nil, "http://example.com" + # # => <a href="http://www.example.com">http://www.example.com</a> + # # You can use a block as well if your link target is hard to fit into the name parameter. ERB example: # # <%= link_to(@profile) do %> diff --git a/actionview/lib/action_view/renderer/streaming_template_renderer.rb b/actionview/lib/action_view/renderer/streaming_template_renderer.rb index ca49eb1144..276a28ce07 100644 --- a/actionview/lib/action_view/renderer/streaming_template_renderer.rb +++ b/actionview/lib/action_view/renderer/streaming_template_renderer.rb @@ -65,7 +65,9 @@ module ActionView yielder = lambda { |*name| view._layout_for(*name) } instrument(:template, identifier: template.identifier, layout: layout.try(:virtual_path)) do + outer_config = I18n.config fiber = Fiber.new do + I18n.config = outer_config if layout layout.render(view, locals, output, &yielder) else diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index 9df2a73448..8a9d7982d3 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -9,7 +9,7 @@ class ApplicationController < ActionController::Base end module Quiz - #Models + # Models Question = Struct.new(:name, :id) do extend ActiveModel::Naming include ActiveModel::Conversion diff --git a/actionview/test/active_record_unit.rb b/actionview/test/active_record_unit.rb index b39ecd8813..7f48b515a0 100644 --- a/actionview/test/active_record_unit.rb +++ b/actionview/test/active_record_unit.rb @@ -38,7 +38,7 @@ class ActiveRecordTestConnector end rescue Exception => e # errors from ActiveRecord setup $stderr.puts "\nSkipping ActiveRecord assertion tests: #{e}" - #$stderr.puts " #{e.backtrace.join("\n ")}\n" + # $stderr.puts " #{e.backtrace.join("\n ")}\n" self.able_to_connect = false end diff --git a/actionview/test/fixtures/layouts/streaming_with_locale.erb b/actionview/test/fixtures/layouts/streaming_with_locale.erb new file mode 100644 index 0000000000..e1fdad2073 --- /dev/null +++ b/actionview/test/fixtures/layouts/streaming_with_locale.erb @@ -0,0 +1,2 @@ +layout.locale: <%= I18n.locale %> +<%= yield %> diff --git a/actionview/test/fixtures/test/streaming_with_locale.erb b/actionview/test/fixtures/test/streaming_with_locale.erb new file mode 100644 index 0000000000..b0f2b2f7e9 --- /dev/null +++ b/actionview/test/fixtures/test/streaming_with_locale.erb @@ -0,0 +1 @@ +view.locale: <%= I18n.locale %> diff --git a/actionview/test/template/date_helper_test.rb b/actionview/test/template/date_helper_test.rb index 5a5550438b..97cfd754be 100644 --- a/actionview/test/template/date_helper_test.rb +++ b/actionview/test/template/date_helper_test.rb @@ -687,7 +687,7 @@ class DateHelperTest < ActionView::TestCase expected << %(<option value=""></option>\n<option value="00">00</option>\n<option value="15">15</option>\n<option value="30">30</option>\n<option value="45">45</option>\n) expected << "</select>\n" - assert_dom_equal expected, select_minute(Time.mktime(2003, 8, 16, 8, 4, 18), include_blank: true , minute_step: 15) + assert_dom_equal expected, select_minute(Time.mktime(2003, 8, 16, 8, 4, 18), include_blank: true, minute_step: 15) end def test_select_minute_nil_with_blank @@ -703,7 +703,7 @@ class DateHelperTest < ActionView::TestCase expected << %(<option value=""></option>\n<option value="00">00</option>\n<option value="15">15</option>\n<option value="30">30</option>\n<option value="45">45</option>\n) expected << "</select>\n" - assert_dom_equal expected, select_minute(nil, include_blank: true , minute_step: 15) + assert_dom_equal expected, select_minute(nil, include_blank: true, minute_step: 15) end def test_select_minute_with_hidden diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index f0eed1e290..82b5b79e82 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -367,7 +367,7 @@ class FormOptionsHelperTest < ActionView::TestCase assert_dom_equal( "<optgroup label=\"----------\"><option value=\"US\">US</option>\n<option value=\"Canada\">Canada</option></optgroup><optgroup label=\"----------\"><option value=\"GB\">GB</option>\n<option value=\"Germany\">Germany</option></optgroup>", - grouped_options_for_select([["US", "Canada"] , ["GB", "Germany"]], nil, divider: "----------") + grouped_options_for_select([["US", "Canada"], ["GB", "Germany"]], nil, divider: "----------") ) end diff --git a/actionview/test/template/number_helper_test.rb b/actionview/test/template/number_helper_test.rb index 2b671a6685..e92bf66203 100644 --- a/actionview/test/template/number_helper_test.rb +++ b/actionview/test/template/number_helper_test.rb @@ -79,7 +79,7 @@ class NumberHelperTest < ActionView::TestCase assert_equal "1.23 <b>km3</b>", number_to_human(1_234_567_000_000, units: volume) assert_equal "1.23 <b>Pl</b>", number_to_human(1_234_567_000_000_000, units: volume) - #Including fractionals + # Including fractionals distance = { mili: "<b>mm</b>", centi: "<b>cm</b>", deci: "<b>dm</b>", unit: "<b>m</b>", ten: "<b>dam</b>", hundred: "<b>hm</b>", thousand: "<b>km</b>", micro: "<b>um</b>", nano: "<b>nm</b>", pico: "<b>pm</b>", femto: "<b>fm</b>" } diff --git a/actionview/test/template/streaming_render_test.rb b/actionview/test/template/streaming_render_test.rb index 23edf7b538..ef000300cc 100644 --- a/actionview/test/template/streaming_render_test.rb +++ b/actionview/test/template/streaming_render_test.rb @@ -5,7 +5,7 @@ require "abstract_unit" class TestController < ActionController::Base end -class FiberedTest < ActiveSupport::TestCase +class SetupFiberedBase < ActiveSupport::TestCase def setup view_paths = ActionController::Base.view_paths @assigns = { secret: "in the sauce", name: nil } @@ -25,7 +25,9 @@ class FiberedTest < ActiveSupport::TestCase end string end +end +class FiberedTest < SetupFiberedBase def test_streaming_works content = [] body = render_body(template: "test/hello_world", layout: "layouts/yield") @@ -111,3 +113,20 @@ class FiberedTest < ActiveSupport::TestCase buffered_render(template: "test/streaming", layout: "layouts/streaming_with_capture") end end + +class FiberedWithLocaleTest < SetupFiberedBase + def setup + @old_locale = I18n.locale + I18n.locale = "da" + super + end + + def teardown + I18n.locale = @old_locale + end + + def test_render_with_streaming_and_locale + assert_equal "layout.locale: da\nview.locale: da\n\n", + buffered_render(template: "test/streaming_with_locale", layout: "layouts/streaming_with_locale") + end +end diff --git a/actionview/test/template/tag_helper_test.rb b/actionview/test/template/tag_helper_test.rb index 8c57803796..f85a6b2f46 100644 --- a/actionview/test/template/tag_helper_test.rb +++ b/actionview/test/template/tag_helper_test.rb @@ -307,8 +307,8 @@ class TagHelperTest < ActionView::TestCase def test_tag_builder_disable_escaping assert_equal '<a href="&"></a>', tag.a(href: "&", escape_attributes: false) - assert_equal '<a href="&">cnt</a>', tag.a(href: "&" , escape_attributes: false) { "cnt" } - assert_equal '<br data-hidden="&">', tag.br("data-hidden": "&" , escape_attributes: false) + assert_equal '<a href="&">cnt</a>', tag.a(href: "&", escape_attributes: false) { "cnt" } + assert_equal '<br data-hidden="&">', tag.br("data-hidden": "&", escape_attributes: false) assert_equal '<a href="&">content</a>', tag.a("content", href: "&", escape_attributes: false) assert_equal '<a href="&">content</a>', tag.a(href: "&", escape_attributes: false) { "content" } end diff --git a/activejob/Rakefile b/activejob/Rakefile index 6f13ef449d..77f3e7f8ff 100644 --- a/activejob/Rakefile +++ b/activejob/Rakefile @@ -2,7 +2,7 @@ require "rake/testtask" -#TODO: add qu back to the list after it support Rails 5.1 +# TODO: add qu back to the list after it support Rails 5.1 ACTIVEJOB_ADAPTERS = %w(async inline delayed_job que queue_classic resque sidekiq sneakers sucker_punch backburner test) ACTIVEJOB_ADAPTERS.delete("queue_classic") if defined?(JRUBY_VERSION) diff --git a/activejob/lib/active_job/core.rb b/activejob/lib/active_job/core.rb index c4e12fc518..879746fc01 100644 --- a/activejob/lib/active_job/core.rb +++ b/activejob/lib/active_job/core.rb @@ -97,17 +97,23 @@ module ActiveJob # ==== Examples # # class DeliverWebhookJob < ActiveJob::Base + # attr_writer :attempt_number + # + # def attempt_number + # @attempt_number ||= 0 + # end + # # def serialize - # super.merge('attempt_number' => (@attempt_number || 0) + 1) + # super.merge('attempt_number' => attempt_number + 1) # end # # def deserialize(job_data) # super - # @attempt_number = job_data['attempt_number'] + # self.attempt_number = job_data['attempt_number'] # end # - # rescue_from(TimeoutError) do |exception| - # raise exception if @attempt_number > 5 + # rescue_from(Timeout::Error) do |exception| + # raise exception if attempt_number > 5 # retry_job(wait: 10) # end # end diff --git a/activejob/lib/active_job/queue_adapters/sidekiq_adapter.rb b/activejob/lib/active_job/queue_adapters/sidekiq_adapter.rb index 5a1135854b..f726e6ad93 100644 --- a/activejob/lib/active_job/queue_adapters/sidekiq_adapter.rb +++ b/activejob/lib/active_job/queue_adapters/sidekiq_adapter.rb @@ -18,7 +18,7 @@ module ActiveJob # Rails.application.config.active_job.queue_adapter = :sidekiq class SidekiqAdapter def enqueue(job) #:nodoc: - #Sidekiq::Client does not support symbols as keys + # Sidekiq::Client does not support symbols as keys job.provider_job_id = Sidekiq::Client.push \ "class" => JobWrapper, "wrapped" => job.class.to_s, diff --git a/activejob/test/support/delayed_job/delayed/backend/test.rb b/activejob/test/support/delayed_job/delayed/backend/test.rb index 9280a37a5c..1691896b7c 100644 --- a/activejob/test/support/delayed_job/delayed/backend/test.rb +++ b/activejob/test/support/delayed_job/delayed/backend/test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -#copied from https://github.com/collectiveidea/delayed_job/blob/master/spec/delayed/backend/test.rb +# copied from https://github.com/collectiveidea/delayed_job/blob/master/spec/delayed/backend/test.rb require "ostruct" # An in-memory backend suitable only for testing. Tries to behave as if it were an ORM. diff --git a/activemodel/lib/active_model/type.rb b/activemodel/lib/active_model/type.rb index 39324999c9..1d7a26fff5 100644 --- a/activemodel/lib/active_model/type.rb +++ b/activemodel/lib/active_model/type.rb @@ -24,7 +24,7 @@ module ActiveModel class << self attr_accessor :registry # :nodoc: - # Add a new type to the registry, allowing it to be get through ActiveModel::Type#lookup + # Add a new type to the registry, allowing it to be gotten through ActiveModel::Type#lookup def register(type_name, klass = nil, **options, &block) registry.register(type_name, klass, **options, &block) end diff --git a/activemodel/lib/active_model/validations/validates.rb b/activemodel/lib/active_model/validations/validates.rb index 43d9f82d9f..e28e7e9219 100644 --- a/activemodel/lib/active_model/validations/validates.rb +++ b/activemodel/lib/active_model/validations/validates.rb @@ -154,7 +154,7 @@ module ActiveModel # When creating custom validators, it might be useful to be able to specify # additional default keys. This can be done by overwriting this method. def _validates_default_keys - [:if, :unless, :on, :allow_blank, :allow_nil , :strict] + [:if, :unless, :on, :allow_blank, :allow_nil, :strict] end def _parse_validates_options(options) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 60ceffac5e..c88a4e7695 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,106 @@ +* Log database query callers + + Add `verbose_query_logs` configuration option to display the caller + of database queries in the log to facilitate N+1 query resolution + and other debugging. Excludes Ruby and Rails callers but not gems. + + Enabled in development only for new and upgraded applications. Not + recommended for use in the production environment since it relies + on Ruby's `Kernel#caller` which is fairly slow. + + *Olivier Lacan* + +* Fix conflicts `counter_cache` with `touch: true` by optimistic locking. + + ``` + # create_table :posts do |t| + # t.integer :comments_count, default: 0 + # t.integer :lock_version + # t.timestamps + # end + class Post < ApplicationRecord + end + + # create_table :comments do |t| + # t.belongs_to :post + # end + class Comment < ApplicationRecord + belongs_to :post, touch: true, counter_cache: true + end + ``` + + Before: + ``` + post = Post.create! + # => begin transaction + INSERT INTO "posts" ("created_at", "updated_at", "lock_version") + VALUES ("2017-12-11 21:27:11.387397", "2017-12-11 21:27:11.387397", 0) + commit transaction + + comment = Comment.create!(post: post) + # => begin transaction + INSERT INTO "comments" ("post_id") VALUES (1) + + UPDATE "posts" SET "comments_count" = COALESCE("comments_count", 0) + 1, + "lock_version" = COALESCE("lock_version", 0) + 1 WHERE "posts"."id" = 1 + + UPDATE "posts" SET "updated_at" = '2017-12-11 21:27:11.398330', + "lock_version" = 1 WHERE "posts"."id" = 1 AND "posts"."lock_version" = 0 + rollback transaction + # => ActiveRecord::StaleObjectError: Attempted to touch a stale object: Post. + + Comment.take.destroy! + # => begin transaction + DELETE FROM "comments" WHERE "comments"."id" = 1 + + UPDATE "posts" SET "comments_count" = COALESCE("comments_count", 0) - 1, + "lock_version" = COALESCE("lock_version", 0) + 1 WHERE "posts"."id" = 1 + + UPDATE "posts" SET "updated_at" = '2017-12-11 21:42:47.785901', + "lock_version" = 1 WHERE "posts"."id" = 1 AND "posts"."lock_version" = 0 + rollback transaction + # => ActiveRecord::StaleObjectError: Attempted to touch a stale object: Post. + ``` + + After: + ``` + post = Post.create! + # => begin transaction + INSERT INTO "posts" ("created_at", "updated_at", "lock_version") + VALUES ("2017-12-11 21:27:11.387397", "2017-12-11 21:27:11.387397", 0) + commit transaction + + comment = Comment.create!(post: post) + # => begin transaction + INSERT INTO "comments" ("post_id") VALUES (1) + + UPDATE "posts" SET "comments_count" = COALESCE("comments_count", 0) + 1, + "lock_version" = COALESCE("lock_version", 0) + 1, + "updated_at" = '2017-12-11 21:37:09.802642' WHERE "posts"."id" = 1 + commit transaction + + comment.destroy! + # => begin transaction + DELETE FROM "comments" WHERE "comments"."id" = 1 + + UPDATE "posts" SET "comments_count" = COALESCE("comments_count", 0) - 1, + "lock_version" = COALESCE("lock_version", 0) + 1, + "updated_at" = '2017-12-11 21:39:02.685520' WHERE "posts"."id" = 1 + commit transaction + ``` + + Fixes #31199. + + *bogdanvlviv* + +* Add support for PostgreSQL operator classes to `add_index`. + + Example: + + add_index :users, :name, using: :gist, opclass: { name: :gist_trgm_ops } + + *Greg Navis* + * Don't allow scopes to be defined which conflict with instance methods on `Relation`. Fixes #31120. @@ -72,7 +175,7 @@ *bogdanvlviv* * Fixed a bug where column orders for an index weren't written to - db/schema.rb when using the sqlite adapter. + `db/schema.rb` when using the sqlite adapter. Fixes #30902. @@ -445,5 +548,4 @@ *Kevin McPhillips* - Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 57c82bf469..591c451da5 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -68,7 +68,7 @@ end (Dir["test/cases/**/*_test.rb"].reject { |x| x.include?("/adapters/") } + Dir["test/cases/adapters/#{adapter_short}/**/*_test.rb"]).all? do |file| - sh(Gem.ruby, "-w" , "-Itest", file) + sh(Gem.ruby, "-w", "-Itest", file) end || raise("Failures") end end diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index ba54cd8f49..68c608df13 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -49,9 +49,9 @@ module ActiveRecord def update_counters(by) if require_counter_update? && foreign_key_present? if target && !stale_target? - target.increment!(reflection.counter_cache_column, by) + target.increment!(reflection.counter_cache_column, by, touch: reflection.options[:touch]) else - klass.update_counters(target_id, reflection.counter_cache_column => by) + klass.update_counters(target_id, reflection.counter_cache_column => by, touch: reflection.options[:touch]) end end end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 9904ee4bed..c161454c1a 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -114,9 +114,13 @@ module ActiveRecord::Associations::Builder # :nodoc: BelongsTo.touch_record(record, record.send(changes_method), foreign_key, n, touch, belongs_to_touch_method) }} - model.after_save callback.(:saved_changes), if: :saved_changes? - model.after_touch callback.(:changes_to_save) - model.after_destroy callback.(:changes_to_save) + unless reflection.counter_cache_column + model.after_create callback.(:saved_changes), if: :saved_changes? + model.after_destroy callback.(:changes_to_save) + end + + model.after_update callback.(:saved_changes), if: :saved_changes? + model.after_touch callback.(:changes_to_save) end def self.add_default_callbacks(model, reflection) diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 0b7c9398a8..35150889d9 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -57,7 +57,7 @@ module ActiveRecord # store_listing = StoreListing.new(price_in_cents: '10.1') # # # before - # store_listing.price_in_cents # => BigDecimal.new(10.1) + # store_listing.price_in_cents # => BigDecimal(10.1) # # class StoreListing < ActiveRecord::Base # attribute :price_in_cents, :integer diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index be2f625d74..0594b4b485 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -6,7 +6,7 @@ module ActiveRecord # this type are typically created and returned by methods in database # adapters. e.g. ActiveRecord::ConnectionAdapters::MySQL::SchemaStatements#indexes class IndexDefinition # :nodoc: - attr_reader :table, :name, :unique, :columns, :lengths, :orders, :where, :type, :using, :comment + attr_reader :table, :name, :unique, :columns, :lengths, :orders, :opclasses, :where, :type, :using, :comment def initialize( table, name, @@ -14,6 +14,7 @@ module ActiveRecord columns = [], lengths: {}, orders: {}, + opclasses: {}, where: nil, type: nil, using: nil, @@ -23,13 +24,23 @@ module ActiveRecord @name = name @unique = unique @columns = columns - @lengths = lengths - @orders = orders + @lengths = concise_options(lengths) + @orders = concise_options(orders) + @opclasses = concise_options(opclasses) @where = where @type = type @using = using @comment = comment end + + private + def concise_options(options) + if columns.size == options.size && options.values.uniq.size == 1 + options.values.first + else + options + end + end end # Abstract representation of a column definition. Instances of this type @@ -85,6 +96,11 @@ module ActiveRecord options[:primary_key] != default_primary_key end + def validate? + options.fetch(:validate, true) + end + alias validated? validate? + def defined_for?(to_table_ord = nil, to_table: nil, **options) if to_table_ord self.to_table == to_table_ord.to_s @@ -204,6 +220,7 @@ module ActiveRecord :decimal, :float, :integer, + :json, :string, :text, :time, diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 9b7345f7c3..4f58b0242c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -600,7 +600,7 @@ module ActiveRecord # to provide these in a migration's +change+ method so it can be reverted. # In that case, +type+ and +options+ will be used by #add_column. def remove_column(table_name, column_name, type = nil, options = {}) - execute "ALTER TABLE #{quote_table_name(table_name)} DROP #{quote_column_name(column_name)}" + execute "ALTER TABLE #{quote_table_name(table_name)} #{remove_column_for_alter(table_name, column_name, type, options)}" end # Changes the column's definition according to the new options. @@ -738,6 +738,28 @@ module ActiveRecord # # Note: only supported by PostgreSQL and MySQL # + # ====== Creating an index with a specific operator class + # + # add_index(:developers, :name, using: 'gist', opclass: :gist_trgm_ops) + # + # generates: + # + # CREATE INDEX developers_on_name ON developers USING gist (name gist_trgm_ops) -- PostgreSQL + # + # add_index(:developers, [:name, :city], using: 'gist', opclass: { city: :gist_trgm_ops }) + # + # generates: + # + # CREATE INDEX developers_on_name_and_city ON developers USING gist (name, city gist_trgm_ops) -- PostgreSQL + # + # add_index(:developers, [:name, :city], using: 'gist', opclass: :gist_trgm_ops) + # + # generates: + # + # CREATE INDEX developers_on_name_and_city ON developers USING gist (name gist_trgm_ops, city gist_trgm_ops) -- PostgreSQL + # + # Note: only supported by PostgreSQL + # # ====== Creating an index with a specific type # # add_index(:developers, :name, type: :fulltext) @@ -942,6 +964,8 @@ module ActiveRecord # Action that happens <tt>ON DELETE</tt>. Valid values are +:nullify+, +:cascade+ and +:restrict+ # [<tt>:on_update</tt>] # Action that happens <tt>ON UPDATE</tt>. Valid values are +:nullify+, +:cascade+ and +:restrict+ + # [<tt>:validate</tt>] + # (Postgres only) Specify whether or not the constraint should be validated. Defaults to +true+. def add_foreign_key(from_table, to_table, options = {}) return unless supports_foreign_keys? @@ -1120,7 +1144,7 @@ module ActiveRecord def add_index_options(table_name, column_name, comment: nil, **options) # :nodoc: column_names = index_column_names(column_name) - options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type) + options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type, :opclass) index_type = options[:type].to_s if options.key?(:type) index_type ||= options[:unique] ? "UNIQUE" : "" @@ -1173,20 +1197,22 @@ module ActiveRecord end def add_index_sort_order(quoted_columns, **options) - if order = options[:order] - case order - when Hash - order = order.symbolize_keys - quoted_columns.each { |name, column| column << " #{order[name].upcase}" if order[name].present? } - when String - quoted_columns.each { |name, column| column << " #{order.upcase}" if order.present? } - end + orders = options_for_index_columns(options[:order]) + quoted_columns.each do |name, column| + column << " #{orders[name].upcase}" if orders[name].present? end + end - quoted_columns + def options_for_index_columns(options) + if options.is_a?(Hash) + options.symbolize_keys + else + Hash.new { |hash, column| hash[column] = options } + end end - # Overridden by the MySQL adapter for supporting index lengths + # Overridden by the MySQL adapter for supporting index lengths and by + # the PostgreSQL adapter for supporting operator classes. def add_options_for_index_columns(quoted_columns, **options) if supports_index_sort_order? quoted_columns = add_index_sort_order(quoted_columns, options) @@ -1340,6 +1366,20 @@ module ActiveRecord options.is_a?(Hash) && options.key?(:name) && options.except(:name, :algorithm).empty? end + def add_column_for_alter(table_name, column_name, type, options = {}) + td = create_table_definition(table_name) + cd = td.new_column_definition(column_name, type, options) + schema_creation.accept(AddColumnDefinition.new(cd)) + end + + def remove_column_for_alter(table_name, column_name, type = nil, options = {}) + "DROP COLUMN #{quote_column_name(column_name)}" + end + + def remove_columns_for_alter(table_name, *column_names) + column_names.map { |column_name| remove_column_for_alter(table_name, column_name) } + end + def insert_versions_sql(versions) sm_table = quote_table_name(ActiveRecord::SchemaMigration.table_name) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 8993c517a6..fc80d332f9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -272,6 +272,11 @@ module ActiveRecord false end + # Does this adapter support creating invalid constraints? + def supports_validate_constraints? + false + end + # Does this adapter support creating foreign key constraints # in the same statement as creating the table? def supports_foreign_keys_in_create? diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index ede8a9c1e2..8569c76317 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -299,7 +299,7 @@ module ActiveRecord def bulk_change_table(table_name, operations) #:nodoc: sqls = operations.flat_map do |command, args| table, arguments = args.shift, args - method = :"#{command}_sql" + method = :"#{command}_for_alter" if respond_to?(method, true) send(method, table, *arguments) @@ -372,11 +372,11 @@ module ActiveRecord end def change_column(table_name, column_name, type, options = {}) #:nodoc: - execute("ALTER TABLE #{quote_table_name(table_name)} #{change_column_sql(table_name, column_name, type, options)}") + execute("ALTER TABLE #{quote_table_name(table_name)} #{change_column_for_alter(table_name, column_name, type, options)}") end def rename_column(table_name, column_name, new_column_name) #:nodoc: - execute("ALTER TABLE #{quote_table_name(table_name)} #{rename_column_sql(table_name, column_name, new_column_name)}") + execute("ALTER TABLE #{quote_table_name(table_name)} #{rename_column_for_alter(table_name, column_name, new_column_name)}") rename_column_indexes(table_name, column_name, new_column_name) end @@ -403,12 +403,13 @@ module ActiveRecord fk.constraint_name AS 'name', rc.update_rule AS 'on_update', rc.delete_rule AS 'on_delete' - FROM information_schema.key_column_usage fk - JOIN information_schema.referential_constraints rc + FROM information_schema.referential_constraints rc + JOIN information_schema.key_column_usage fk USING (constraint_schema, constraint_name) WHERE fk.referenced_column_name IS NOT NULL AND fk.table_schema = #{scope[:schema]} AND fk.table_name = #{scope[:name]} + AND rc.constraint_schema = #{scope[:schema]} AND rc.table_name = #{scope[:name]} SQL @@ -605,25 +606,6 @@ module ActiveRecord end end - def add_index_length(quoted_columns, **options) - if length = options[:length] - case length - when Hash - length = length.symbolize_keys - quoted_columns.each { |name, column| column << "(#{length[name]})" if length[name].present? } - when Integer - quoted_columns.each { |name, column| column << "(#{length})" } - end - end - - quoted_columns - end - - def add_options_for_index_columns(quoted_columns, **options) - quoted_columns = add_index_length(quoted_columns, options) - super - end - # See https://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html ER_DUP_ENTRY = 1062 ER_NOT_NULL_VIOLATION = 1048 @@ -671,13 +653,7 @@ module ActiveRecord end end - def add_column_sql(table_name, column_name, type, options = {}) - td = create_table_definition(table_name) - cd = td.new_column_definition(column_name, type, options) - schema_creation.accept(AddColumnDefinition.new(cd)) - end - - def change_column_sql(table_name, column_name, type, options = {}) + def change_column_for_alter(table_name, column_name, type, options = {}) column = column_for(table_name, column_name) type ||= column.sql_type @@ -698,7 +674,7 @@ module ActiveRecord schema_creation.accept(ChangeColumnDefinition.new(cd, column.name)) end - def rename_column_sql(table_name, column_name, new_column_name) + def rename_column_for_alter(table_name, column_name, new_column_name) column = column_for(table_name, column_name) options = { default: column.default, @@ -712,31 +688,23 @@ module ActiveRecord schema_creation.accept(ChangeColumnDefinition.new(cd, column.name)) end - def remove_column_sql(table_name, column_name, type = nil, options = {}) - "DROP #{quote_column_name(column_name)}" - end - - def remove_columns_sql(table_name, *column_names) - column_names.map { |column_name| remove_column_sql(table_name, column_name) } - end - - def add_index_sql(table_name, column_name, options = {}) + def add_index_for_alter(table_name, column_name, options = {}) index_name, index_type, index_columns, _, index_algorithm, index_using = add_index_options(table_name, column_name, options) index_algorithm[0, 0] = ", " if index_algorithm.present? "ADD #{index_type} INDEX #{quote_column_name(index_name)} #{index_using} (#{index_columns})#{index_algorithm}" end - def remove_index_sql(table_name, options = {}) + def remove_index_for_alter(table_name, options = {}) index_name = index_name_for_remove(table_name, options) "DROP INDEX #{quote_column_name(index_name)}" end - def add_timestamps_sql(table_name, options = {}) - [add_column_sql(table_name, :created_at, :datetime, options), add_column_sql(table_name, :updated_at, :datetime, options)] + def add_timestamps_for_alter(table_name, options = {}) + [add_column_for_alter(table_name, :created_at, :datetime, options), add_column_for_alter(table_name, :updated_at, :datetime, options)] end - def remove_timestamps_sql(table_name, options = {}) - [remove_column_sql(table_name, :updated_at), remove_column_sql(table_name, :created_at)] + def remove_timestamps_for_alter(table_name, options = {}) + [remove_column_for_alter(table_name, :updated_at), remove_column_for_alter(table_name, :created_at)] end # MySQL is too stupid to create a temporary table for use subquery, so we have diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb index da25e4863c..2ed4ad16ae 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb @@ -32,10 +32,6 @@ module ActiveRecord args.each { |name| column(name, :longtext, options) } end - def json(*args, **options) - args.each { |name| column(name, :json, options) } - end - def unsigned_integer(*args, **options) args.each { |name| column(name, :unsigned_integer, options) } end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index a15c7d1787..ce50590651 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -22,23 +22,26 @@ module ActiveRecord index_using = mysql_index_type end - indexes << IndexDefinition.new( + indexes << [ row[:Table], row[:Key_name], row[:Non_unique].to_i == 0, + [], + lengths: {}, + orders: {}, type: index_type, using: index_using, comment: row[:Index_comment].presence - ) + ] end - indexes.last.columns << row[:Column_name] - indexes.last.lengths.merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part] - indexes.last.orders.merge!(row[:Column_name] => :desc) if row[:Collation] == "D" + indexes.last[-2] << row[:Column_name] + indexes.last[-1][:lengths].merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part] + indexes.last[-1][:orders].merge!(row[:Column_name] => :desc) if row[:Collation] == "D" end end - indexes + indexes.map { |index| IndexDefinition.new(*index) } end def remove_column(table_name, column_name, type = nil, options = {}) @@ -103,6 +106,18 @@ module ActiveRecord super unless specifier == "RESTRICT" end + def add_index_length(quoted_columns, **options) + lengths = options_for_index_columns(options[:length]) + quoted_columns.each do |name, column| + column << "(#{lengths[name]})" if lengths[name].present? + end + end + + def add_options_for_index_columns(quoted_columns, **options) + quoted_columns = add_index_length(quoted_columns, options) + super + end + def data_source_sql(name = nil, type: nil) scope = quoted_scope(name, type: type) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/decimal.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/decimal.rb index 879dba7afd..e7d33855c4 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/decimal.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/decimal.rb @@ -6,7 +6,7 @@ module ActiveRecord module OID # :nodoc: class Decimal < Type::Decimal # :nodoc: def infinity(options = {}) - BigDecimal.new("Infinity") * (options[:negative] ? -1 : 1) + BigDecimal("Infinity") * (options[:negative] ? -1 : 1) end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb index 59f661da25..8e381a92cf 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb @@ -5,6 +5,18 @@ module ActiveRecord module PostgreSQL class SchemaCreation < AbstractAdapter::SchemaCreation # :nodoc: private + def visit_AlterTable(o) + super << o.constraint_validations.map { |fk| visit_ValidateConstraint fk }.join(" ") + end + + def visit_AddForeignKey(o) + super.dup.tap { |sql| sql << " NOT VALID" unless o.validate? } + end + + def visit_ValidateConstraint(name) + "VALIDATE CONSTRAINT #{quote_column_name(name)}" + end + def add_column_options!(sql, options) if options[:collation] sql << " COLLATE \"#{options[:collation]}\"" diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb index 75622eb304..6047217fcd 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb @@ -95,10 +95,6 @@ module ActiveRecord args.each { |name| column(name, :int8range, options) } end - def json(*args, **options) - args.each { |name| column(name, :json, options) } - end - def jsonb(*args, **options) args.each { |name| column(name, :jsonb, options) } end @@ -192,6 +188,19 @@ module ActiveRecord class Table < ActiveRecord::ConnectionAdapters::Table include ColumnMethods end + + class AlterTable < ActiveRecord::ConnectionAdapters::AlterTable + attr_reader :constraint_validations + + def initialize(td) + super + @constraint_validations = [] + end + + def validate_constraint(name) + @constraint_validations << name + end + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 846e721983..bf5fbb30e1 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -87,10 +87,7 @@ module ActiveRecord result = query(<<-SQL, "SCHEMA") SELECT distinct i.relname, d.indisunique, d.indkey, pg_get_indexdef(d.indexrelid), t.oid, - pg_catalog.obj_description(i.oid, 'pg_class') AS comment, - (SELECT COUNT(*) FROM pg_opclass o - JOIN (SELECT unnest(string_to_array(d.indclass::text, ' '))::int oid) c - ON o.oid = c.oid WHERE o.opcdefault = 'f') + pg_catalog.obj_description(i.oid, 'pg_class') AS comment FROM pg_class t INNER JOIN pg_index d ON t.oid = d.indrelid INNER JOIN pg_class i ON d.indexrelid = i.oid @@ -109,11 +106,13 @@ module ActiveRecord inddef = row[3] oid = row[4] comment = row[5] - opclass = row[6] using, expressions, where = inddef.scan(/ USING (\w+?) \((.+?)\)(?: WHERE (.+))?\z/).flatten - if indkey.include?(0) || opclass > 0 + orders = {} + opclasses = {} + + if indkey.include?(0) columns = expressions else columns = Hash[query(<<-SQL.strip_heredoc, "SCHEMA")].values_at(*indkey).compact @@ -123,10 +122,12 @@ module ActiveRecord AND a.attnum IN (#{indkey.join(",")}) SQL - # add info on sort order for columns (only desc order is explicitly specified, asc is the default) - orders = Hash[ - expressions.scan(/(\w+) DESC/).flatten.map { |order_column| [order_column, :desc] } - ] + # add info on sort order (only desc order is explicitly specified, asc is the default) + # and non-default opclasses + expressions.scan(/(\w+)(?: (?!DESC)(\w+))?(?: (DESC))?/).each do |column, opclass, desc| + opclasses[column] = opclass.to_sym if opclass + orders[column] = :desc if desc + end end IndexDefinition.new( @@ -135,6 +136,7 @@ module ActiveRecord unique, columns, orders: orders, + opclasses: opclasses, where: where, using: using.to_sym, comment: comment.presence @@ -392,50 +394,23 @@ module ActiveRecord def change_column(table_name, column_name, type, options = {}) #:nodoc: clear_cache! - quoted_table_name = quote_table_name(table_name) - quoted_column_name = quote_column_name(column_name) - sql_type = type_to_sql(type, options) - sql = "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quoted_column_name} TYPE #{sql_type}".dup - if options[:collation] - sql << " COLLATE \"#{options[:collation]}\"" - end - if options[:using] - sql << " USING #{options[:using]}" - elsif options[:cast_as] - cast_as_type = type_to_sql(options[:cast_as], options) - sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})" - end - execute sql - - change_column_default(table_name, column_name, options[:default]) if options.key?(:default) - change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) - change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) + sqls, procs = change_column_for_alter(table_name, column_name, type, options) + execute "ALTER TABLE #{quote_table_name(table_name)} #{sqls.join(", ")}" + procs.each(&:call) end # Changes the default value of a table column. def change_column_default(table_name, column_name, default_or_changes) # :nodoc: - clear_cache! - column = column_for(table_name, column_name) - return unless column - - default = extract_new_default_value(default_or_changes) - alter_column_query = "ALTER TABLE #{quote_table_name(table_name)} ALTER COLUMN #{quote_column_name(column_name)} %s" - if default.nil? - # <tt>DEFAULT NULL</tt> results in the same behavior as <tt>DROP DEFAULT</tt>. However, PostgreSQL will - # cast the default to the columns type, which leaves us with a default like "default NULL::character varying". - execute alter_column_query % "DROP DEFAULT" - else - execute alter_column_query % "SET DEFAULT #{quote_default_expression(default, column)}" - end + execute "ALTER TABLE #{quote_table_name(table_name)} #{change_column_default_for_alter(table_name, column_name, default_or_changes)}" end def change_column_null(table_name, column_name, null, default = nil) #:nodoc: clear_cache! unless null || default.nil? column = column_for(table_name, column_name) - execute("UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote_default_expression(default, column)} WHERE #{quote_column_name(column_name)} IS NULL") if column + execute "UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote_default_expression(default, column)} WHERE #{quote_column_name(column_name)} IS NULL" if column end - execute("ALTER TABLE #{quote_table_name(table_name)} ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL") + execute "ALTER TABLE #{quote_table_name(table_name)} #{change_column_null_for_alter(table_name, column_name, null, default)}" end # Adds comment for given table column or drops it if +comment+ is a +nil+ @@ -458,8 +433,8 @@ module ActiveRecord end def add_index(table_name, column_name, options = {}) #:nodoc: - index_name, index_type, index_columns, index_options, index_algorithm, index_using, comment = add_index_options(table_name, column_name, options) - execute("CREATE #{index_type} INDEX #{index_algorithm} #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} #{index_using} (#{index_columns})#{index_options}").tap do + index_name, index_type, index_columns_and_opclasses, index_options, index_algorithm, index_using, comment = add_index_options(table_name, column_name, options) + execute("CREATE #{index_type} INDEX #{index_algorithm} #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} #{index_using} (#{index_columns_and_opclasses})#{index_options}").tap do execute "COMMENT ON INDEX #{quote_column_name(index_name)} IS #{quote(comment)}" if comment end end @@ -499,7 +474,7 @@ module ActiveRecord def foreign_keys(table_name) scope = quoted_scope(table_name) fk_info = exec_query(<<-SQL.strip_heredoc, "SCHEMA") - SELECT t2.oid::regclass::text AS to_table, a1.attname AS column, a2.attname AS primary_key, c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete + SELECT t2.oid::regclass::text AS to_table, a1.attname AS column, a2.attname AS primary_key, c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete, c.convalidated AS valid FROM pg_constraint c JOIN pg_class t1 ON c.conrelid = t1.oid JOIN pg_class t2 ON c.confrelid = t2.oid @@ -521,6 +496,7 @@ module ActiveRecord options[:on_delete] = extract_foreign_key_action(row["on_delete"]) options[:on_update] = extract_foreign_key_action(row["on_update"]) + options[:validate] = row["valid"] ForeignKeyDefinition.new(table_name, row["to_table"], options) end @@ -581,6 +557,43 @@ module ActiveRecord PostgreSQL::SchemaDumper.create(self, options) end + # Validates the given constraint. + # + # Validates the constraint named +constraint_name+ on +accounts+. + # + # validate_constraint :accounts, :constraint_name + def validate_constraint(table_name, constraint_name) + return unless supports_validate_constraints? + + at = create_alter_table table_name + at.validate_constraint constraint_name + + execute schema_creation.accept(at) + end + + # Validates the given foreign key. + # + # Validates the foreign key on +accounts.branch_id+. + # + # validate_foreign_key :accounts, :branches + # + # Validates the foreign key on +accounts.owner_id+. + # + # validate_foreign_key :accounts, column: :owner_id + # + # Validates the foreign key named +special_fk_name+ on the +accounts+ table. + # + # validate_foreign_key :accounts, name: :special_fk_name + # + # The +options+ hash accepts the same keys as SchemaStatements#add_foreign_key. + def validate_foreign_key(from_table, options_or_to_table = {}) + return unless supports_validate_constraints? + + fk_name_to_validate = foreign_key_for!(from_table, options_or_to_table).name + + validate_constraint from_table, fk_name_to_validate + end + private def schema_creation PostgreSQL::SchemaCreation.new(self) @@ -590,6 +603,10 @@ module ActiveRecord PostgreSQL::TableDefinition.new(*args) end + def create_alter_table(name) + PostgreSQL::AlterTable.new create_table_definition(name) + end + def new_column_from_field(table_name, field) column_name, type, default, notnull, oid, fmod, collation, comment = field type_metadata = fetch_type_metadata(column_name, type, oid.to_i, fmod.to_i) @@ -629,6 +646,66 @@ module ActiveRecord end end + def change_column_sql(table_name, column_name, type, options = {}) + quoted_column_name = quote_column_name(column_name) + sql_type = type_to_sql(type, options) + sql = "ALTER COLUMN #{quoted_column_name} TYPE #{sql_type}".dup + if options[:collation] + sql << " COLLATE \"#{options[:collation]}\"" + end + if options[:using] + sql << " USING #{options[:using]}" + elsif options[:cast_as] + cast_as_type = type_to_sql(options[:cast_as], options) + sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})" + end + + sql + end + + def change_column_for_alter(table_name, column_name, type, options = {}) + sqls = [change_column_sql(table_name, column_name, type, options)] + procs = [] + sqls << change_column_default_for_alter(table_name, column_name, options[:default]) if options.key?(:default) + sqls << change_column_null_for_alter(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + procs << Proc.new { change_column_comment(table_name, column_name, options[:comment]) } if options.key?(:comment) + + [sqls, procs] + end + + + # Changes the default value of a table column. + def change_column_default_for_alter(table_name, column_name, default_or_changes) # :nodoc: + column = column_for(table_name, column_name) + return unless column + + default = extract_new_default_value(default_or_changes) + alter_column_query = "ALTER COLUMN #{quote_column_name(column_name)} %s" + if default.nil? + # <tt>DEFAULT NULL</tt> results in the same behavior as <tt>DROP DEFAULT</tt>. However, PostgreSQL will + # cast the default to the columns type, which leaves us with a default like "default NULL::character varying". + alter_column_query % "DROP DEFAULT" + else + alter_column_query % "SET DEFAULT #{quote_default_expression(default, column)}" + end + end + + def change_column_null_for_alter(table_name, column_name, null, default = nil) #:nodoc: + "ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL" + end + + def add_index_opclass(quoted_columns, **options) + opclasses = options_for_index_columns(options[:opclass]) + quoted_columns.each do |name, column| + column << " #{opclasses[name]}" if opclasses[name].present? + end + end + + def add_options_for_index_columns(quoted_columns, **options) + quoted_columns = add_index_opclass(quoted_columns, options) + super + end + def data_source_sql(name = nil, type: nil) scope = quoted_scope(name, type: type) scope[:type] ||= "'r','v','m'" # (r)elation/table, (v)iew, (m)aterialized view diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 27011bfe92..23fc69d649 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -142,6 +142,10 @@ module ActiveRecord true end + def supports_validate_constraints? + true + end + def supports_views? true end @@ -386,7 +390,6 @@ module ActiveRecord end private - # See https://www.postgresql.org/docs/current/static/errcodes-appendix.html VALUE_LIMIT_VIOLATION = "22001" NUMERIC_VALUE_OUT_OF_RANGE = "22003" diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index daece2bffd..c72db15ce3 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -70,7 +70,8 @@ module ActiveRecord time: { name: "time" }, date: { name: "date" }, binary: { name: "blob" }, - boolean: { name: "boolean" } + boolean: { name: "boolean" }, + json: { name: "json" }, } ## @@ -134,6 +135,10 @@ module ActiveRecord true end + def supports_json? + true + end + def supports_multi_insert? sqlite_version >= "3.7.11" end @@ -369,6 +374,10 @@ module ActiveRecord end private + def initialize_type_map(m = type_map) + super + register_class_with_limit m, %r(int)i, SQLite3Integer + end def table_structure(table_name) structure = exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", "SCHEMA") @@ -398,18 +407,21 @@ module ActiveRecord options[:id] = false create_table(to, options) do |definition| @definition = definition - @definition.primary_key(from_primary_key) if from_primary_key.present? + if from_primary_key.is_a?(Array) + @definition.primary_keys from_primary_key + end columns(from).each do |column| column_name = options[:rename] ? (options[:rename][column.name] || options[:rename][column.name.to_sym] || column.name) : column.name - next if column_name == from_primary_key @definition.column(column_name, column.type, limit: column.limit, default: column.default, precision: column.precision, scale: column.scale, - null: column.null, collation: column.collation) + null: column.null, collation: column.collation, + primary_key: column_name == from_primary_key + ) end yield @definition if block_given? end @@ -422,6 +434,9 @@ module ActiveRecord def copy_table_indexes(from, to, rename = {}) indexes(from).each do |index| name = index.name + # indexes sqlite creates for internal use start with `sqlite_` and + # don't need to be copied + next if name.starts_with?("sqlite_") if to == "a#{from}" name = "t#{name}" elsif from == "a#{to}" @@ -524,6 +539,17 @@ module ActiveRecord def configure_connection execute("PRAGMA foreign_keys = ON", "SCHEMA") end + + class SQLite3Integer < Type::Integer # :nodoc: + private + def _limit + # INTEGER storage class can be stored 8 bytes value. + # See https://www.sqlite.org/datatype3.html#storage_classes_and_datatypes + limit || 8 + end + end + + ActiveRecord::Type.register(:integer, SQLite3Integer, adapter: :sqlite3) end ActiveSupport.run_load_hooks(:active_record_sqlite3adapter, SQLite3Adapter) end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 481159e501..88810cb328 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -18,6 +18,13 @@ module ActiveRecord mattr_accessor :logger, instance_writer: false ## + # :singleton-method: + # + # Specifies if the methods calling database queries should be logged below + # their relevant queries. Defaults to false. + mattr_accessor :verbose_query_logs, instance_writer: false, default: false + + ## # Contains the database configuration - as is typically stored in config/database.yml - # as a Hash. # diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 0715c11473..f3fe610c09 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -47,14 +47,13 @@ module ActiveRecord # Determines if one of the attributes passed in is the inheritance column, # and if the inheritance column is attr accessible, it initializes an # instance of the given subclass instead of the base class. - def new(*args, &block) + def new(attributes = nil, &block) if abstract_class? || self == Base raise NotImplementedError, "#{self} is an abstract class and cannot be instantiated." end - attrs = args.first if has_attribute?(inheritance_column) - subclass = subclass_from_attributes(attrs) + subclass = subclass_from_attributes(attributes) if subclass.nil? && base_class == self subclass = subclass_from_attributes(column_defaults) @@ -62,7 +61,7 @@ module ActiveRecord end if subclass && subclass != self - subclass.new(*args, &block) + subclass.new(attributes, &block) else super end diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index 405f3a30c6..ba3419be6c 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -90,6 +90,48 @@ module ActiveRecord def logger ActiveRecord::Base.logger end + + def debug(progname = nil, &block) + return unless super + + if ActiveRecord::Base.verbose_query_logs + log_query_source + end + end + + def log_query_source + source_line, line_number = extract_callstack(caller_locations) + + if source_line + if defined?(::Rails.root) + app_root = "#{::Rails.root.to_s}/".freeze + source_line = source_line.sub(app_root, "") + end + + logger.debug(" ↳ #{ source_line }:#{ line_number }") + end + end + + def extract_callstack(callstack) + line = callstack.find do |frame| + frame.absolute_path && !ignored_callstack(frame.absolute_path) + end + + offending_line = line || callstack.first + + [ + offending_line.path, + offending_line.lineno, + offending_line.label + ] + end + + RAILS_GEM_ROOT = File.expand_path("../../../..", __FILE__) + "/" + + def ignored_callstack(path) + path.start_with?(RAILS_GEM_ROOT) || + path.start_with?(RbConfig::CONFIG["rubylibdir"]) + end end end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 15e9c09ffb..f6648a4e3d 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1045,7 +1045,7 @@ module ActiveRecord new(:up, migrations(migrations_paths), nil) end - def get_all_versions(connection = Base.connection) + def get_all_versions if SchemaMigration.table_exists? SchemaMigration.all_versions.map(&:to_i) else @@ -1053,12 +1053,13 @@ module ActiveRecord end end - def current_version(connection = Base.connection) - get_all_versions(connection).max || 0 + def current_version(connection = nil) + get_all_versions.max || 0 + rescue ActiveRecord::NoDatabaseError end - def needs_migration?(connection = Base.connection) - (migrations(migrations_paths).collect(&:version) - get_all_versions(connection)).size > 0 + def needs_migration?(connection = nil) + (migrations(migrations_paths).collect(&:version) - get_all_versions).size > 0 end def any_migrations? @@ -1276,10 +1277,10 @@ module ActiveRecord end def validate(migrations) - name , = migrations.group_by(&:name).find { |_, v| v.length > 1 } + name, = migrations.group_by(&:name).find { |_, v| v.length > 1 } raise DuplicateMigrationNameError.new(name) if name - version , = migrations.group_by(&:version).find { |_, v| v.length > 1 } + version, = migrations.group_by(&:version).find { |_, v| v.length > 1 } raise DuplicateMigrationVersionError.new(version) if version end diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index c979aaf0a0..bd8c054c28 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -16,6 +16,18 @@ module ActiveRecord V5_2 = Current class V5_1 < V5_2 + def change_column(table_name, column_name, type, options = {}) + if adapter_name == "PostgreSQL" + clear_cache! + sql = connection.send(:change_column_sql, table_name, column_name, type, options) + execute "ALTER TABLE #{quote_table_name(table_name)} #{sql}" + change_column_default(table_name, column_name, options[:default]) if options.key?(:default) + change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) + else + super + end + end end class V5_0 < V5_1 diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 1941d3d5ea..773b0f6cde 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -87,19 +87,6 @@ module ActiveRecord # Sets the name of the internal metadata table. ## - # :singleton-method: protected_environments - # :call-seq: protected_environments - # - # The array of names of environments where destructive actions should be prohibited. By default, - # the value is <tt>["production"]</tt>. - - ## - # :singleton-method: protected_environments= - # :call-seq: protected_environments=(environments) - # - # Sets an array of names of environments where destructive actions should be prohibited. - - ## # :singleton-method: pluralize_table_names # :call-seq: pluralize_table_names # @@ -122,9 +109,9 @@ module ActiveRecord class_attribute :table_name_suffix, instance_writer: false, default: "" class_attribute :schema_migrations_table_name, instance_accessor: false, default: "schema_migrations" class_attribute :internal_metadata_table_name, instance_accessor: false, default: "ar_internal_metadata" - class_attribute :protected_environments, instance_accessor: false, default: [ "production" ] class_attribute :pluralize_table_names, instance_writer: false, default: true + self.protected_environments = ["production"] self.inheritance_column = "type" self.ignored_columns = [].freeze @@ -238,6 +225,21 @@ module ActiveRecord (parents.detect { |p| p.respond_to?(:table_name_suffix) } || self).table_name_suffix end + # The array of names of environments where destructive actions should be prohibited. By default, + # the value is <tt>["production"]</tt>. + def protected_environments + if defined?(@protected_environments) + @protected_environments + else + superclass.protected_environments + end + end + + # Sets an array of names of environments where destructive actions should be prohibited. + def protected_environments=(environments) + @protected_environments = environments.map(&:to_s) + end + # Defines the name of the table column which will store the class name on single-table # inheritance situations. # diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 4e1b05dbf6..a13b0d0181 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -99,7 +99,9 @@ module ActiveRecord # for updating all records in a single query. def update(id = :all, attributes) if id.is_a?(Array) - id.map.with_index { |one_id, idx| update(one_id, attributes[idx]) }.compact + id.map { |one_id| find(one_id) }.each_with_index { |object, idx| + object.update(attributes[idx]) + } elsif id == :all all.each { |record| record.update(attributes) } else @@ -112,7 +114,6 @@ module ActiveRecord object.update(attributes) object end - rescue RecordNotFound end # Destroy an object (or multiple objects) that has the given id. The object is instantiated first, @@ -136,11 +137,10 @@ module ActiveRecord # Todo.destroy(todos) def destroy(id) if id.is_a?(Array) - id.map { |one_id| destroy(one_id) }.compact + find(id).each(&:destroy) else find(id).destroy end - rescue RecordNotFound end # Deletes the row with a primary key matching the +id+ argument, using a diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 9ee8425e1b..4538ed6a5f 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -90,12 +90,15 @@ module ActiveRecord filename = File.join(app.config.paths["db"].first, "schema_cache.yml") if File.file?(filename) + current_version = ActiveRecord::Migrator.current_version + next if current_version.nil? + cache = YAML.load(File.read(filename)) - if cache.version == ActiveRecord::Migrator.current_version + if cache.version == current_version connection.schema_cache = cache connection_pool.schema_cache = cache.dup else - warn "Ignoring db/schema_cache.yml because it has expired. The current schema version is #{ActiveRecord::Migrator.current_version}, but the one in the cache is #{cache.version}." + warn "Ignoring db/schema_cache.yml because it has expired. The current schema version is #{current_version}, but the one in the cache is #{cache.version}." end end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 081ef5771f..dd0d52ad1c 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -52,8 +52,8 @@ module ActiveRecord # # user = users.new { |user| user.name = 'Oscar' } # user.name # => Oscar - def new(*args, &block) - scoping { @klass.new(*args, &block) } + def new(attributes = nil, &block) + scoping { klass.new(scope_for_create(attributes), &block) } end alias build new @@ -77,8 +77,12 @@ module ActiveRecord # # users.create(name: nil) # validation on name # # => #<User id: nil, name: nil, ...> - def create(*args, &block) - scoping { @klass.create(*args, &block) } + def create(attributes = nil, &block) + if attributes.is_a?(Array) + attributes.collect { |attr| create(attr, &block) } + else + scoping { klass.create(scope_for_create(attributes), &block) } + end end # Similar to #create, but calls @@ -87,8 +91,12 @@ module ActiveRecord # # Expects arguments in the same format as # {ActiveRecord::Base.create!}[rdoc-ref:Persistence::ClassMethods#create!]. - def create!(*args, &block) - scoping { @klass.create!(*args, &block) } + def create!(attributes = nil, &block) + if attributes.is_a?(Array) + attributes.collect { |attr| create!(attr, &block) } + else + scoping { klass.create!(scope_for_create(attributes), &block) } + end end def first_or_create(attributes = nil, &block) # :nodoc: @@ -306,7 +314,7 @@ module ActiveRecord stmt = Arel::UpdateManager.new - stmt.set Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates)) + stmt.set Arel.sql(@klass.sanitize_sql_for_assignment(updates)) stmt.table(table) if has_join_values? @@ -440,8 +448,10 @@ module ActiveRecord where_clause.to_h(relation_table_name) end - def scope_for_create - where_values_hash.merge!(create_with_value.stringify_keys) + def scope_for_create(attributes = nil) + scope = where_values_hash.merge!(create_with_value.stringify_keys) + scope.merge!(attributes) if attributes + scope end # Returns true if relation needs eager loading. diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 749223422f..6b3ff3d610 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1041,7 +1041,7 @@ module ActiveRecord if select_values.any? arel.project(*arel_columns(select_values.uniq)) elsif @klass.ignored_columns.any? - arel.project(*arel_columns(@klass.column_names)) + arel.project(*arel_columns(@klass.column_names.map(&:to_sym))) else arel.project(table[Arel.star]) end @@ -1121,7 +1121,7 @@ module ActiveRecord def preprocess_order_args(order_args) order_args.map! do |arg| - klass.send(:sanitize_sql_for_order, arg) + klass.sanitize_sql_for_order(arg) end order_args.flatten! diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index 752bb38481..a502713e56 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -47,7 +47,7 @@ module ActiveRecord end def to_h(table_name = nil) - equalities = predicates.grep(Arel::Nodes::Equality) + equalities = equalities(predicates) if table_name equalities = equalities.select do |node| node.left.relation.name == table_name @@ -90,6 +90,20 @@ module ActiveRecord end private + def equalities(predicates) + equalities = [] + + predicates.each do |node| + case node + when Arel::Nodes::Equality + equalities << node + when Arel::Nodes::And + equalities.concat equalities(node.children) + end + end + + equalities + end def predicates_unreferenced_by(other) predicates.reject do |n| @@ -121,7 +135,7 @@ module ActiveRecord end def except_predicates(columns) - self.predicates.reject do |node| + predicates.reject do |node| case node when Arel::Nodes::Between, Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality, Arel::Nodes::NotEqual, Arel::Nodes::LessThan, Arel::Nodes::LessThanOrEqual, Arel::Nodes::GreaterThan, Arel::Nodes::GreaterThanOrEqual subrelation = (node.left.kind_of?(Arel::Attributes::Attribute) ? node.left : node.right) diff --git a/activerecord/lib/active_record/relation/where_clause_factory.rb b/activerecord/lib/active_record/relation/where_clause_factory.rb index 1374785354..4ae94f4bfe 100644 --- a/activerecord/lib/active_record/relation/where_clause_factory.rb +++ b/activerecord/lib/active_record/relation/where_clause_factory.rb @@ -11,7 +11,7 @@ module ActiveRecord def build(opts, other) case opts when String, Array - parts = [klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))] + parts = [klass.sanitize_sql(other.empty? ? opts : ([opts] + other))] when Hash attributes = predicate_builder.resolve_column_aliases(opts) attributes = klass.send(:expand_hash_conditions_for_aggregates, attributes) diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 21f8bc7cb2..58da106092 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -5,80 +5,135 @@ module ActiveRecord extend ActiveSupport::Concern module ClassMethods - private - - # Accepts an array or string of SQL conditions and sanitizes - # them into a valid SQL fragment for a WHERE clause. - # - # sanitize_sql_for_conditions(["name=? and group_id=?", "foo'bar", 4]) - # # => "name='foo''bar' and group_id=4" - # - # sanitize_sql_for_conditions(["name=:name and group_id=:group_id", name: "foo'bar", group_id: 4]) - # # => "name='foo''bar' and group_id='4'" - # - # sanitize_sql_for_conditions(["name='%s' and group_id='%s'", "foo'bar", 4]) - # # => "name='foo''bar' and group_id='4'" - # - # sanitize_sql_for_conditions("name='foo''bar' and group_id='4'") - # # => "name='foo''bar' and group_id='4'" - def sanitize_sql_for_conditions(condition) # :doc: - return nil if condition.blank? - - case condition - when Array; sanitize_sql_array(condition) - else condition - end + # Accepts an array or string of SQL conditions and sanitizes + # them into a valid SQL fragment for a WHERE clause. + # + # sanitize_sql_for_conditions(["name=? and group_id=?", "foo'bar", 4]) + # # => "name='foo''bar' and group_id=4" + # + # sanitize_sql_for_conditions(["name=:name and group_id=:group_id", name: "foo'bar", group_id: 4]) + # # => "name='foo''bar' and group_id='4'" + # + # sanitize_sql_for_conditions(["name='%s' and group_id='%s'", "foo'bar", 4]) + # # => "name='foo''bar' and group_id='4'" + # + # sanitize_sql_for_conditions("name='foo''bar' and group_id='4'") + # # => "name='foo''bar' and group_id='4'" + def sanitize_sql_for_conditions(condition) + return nil if condition.blank? + + case condition + when Array; sanitize_sql_array(condition) + else condition end - alias :sanitize_sql :sanitize_sql_for_conditions - - # Accepts an array, hash, or string of SQL conditions and sanitizes - # them into a valid SQL fragment for a SET clause. - # - # sanitize_sql_for_assignment(["name=? and group_id=?", nil, 4]) - # # => "name=NULL and group_id=4" - # - # sanitize_sql_for_assignment(["name=:name and group_id=:group_id", name: nil, group_id: 4]) - # # => "name=NULL and group_id=4" - # - # Post.send(:sanitize_sql_for_assignment, { name: nil, group_id: 4 }) - # # => "`posts`.`name` = NULL, `posts`.`group_id` = 4" - # - # sanitize_sql_for_assignment("name=NULL and group_id='4'") - # # => "name=NULL and group_id='4'" - def sanitize_sql_for_assignment(assignments, default_table_name = table_name) # :doc: - case assignments - when Array; sanitize_sql_array(assignments) - when Hash; sanitize_sql_hash_for_assignment(assignments, default_table_name) - else assignments - end + end + alias :sanitize_sql :sanitize_sql_for_conditions + + # Accepts an array, hash, or string of SQL conditions and sanitizes + # them into a valid SQL fragment for a SET clause. + # + # sanitize_sql_for_assignment(["name=? and group_id=?", nil, 4]) + # # => "name=NULL and group_id=4" + # + # sanitize_sql_for_assignment(["name=:name and group_id=:group_id", name: nil, group_id: 4]) + # # => "name=NULL and group_id=4" + # + # Post.sanitize_sql_for_assignment({ name: nil, group_id: 4 }) + # # => "`posts`.`name` = NULL, `posts`.`group_id` = 4" + # + # sanitize_sql_for_assignment("name=NULL and group_id='4'") + # # => "name=NULL and group_id='4'" + def sanitize_sql_for_assignment(assignments, default_table_name = table_name) + case assignments + when Array; sanitize_sql_array(assignments) + when Hash; sanitize_sql_hash_for_assignment(assignments, default_table_name) + else assignments end - - # Accepts an array, or string of SQL conditions and sanitizes - # them into a valid SQL fragment for an ORDER clause. - # - # sanitize_sql_for_order(["field(id, ?)", [1,3,2]]) - # # => "field(id, 1,3,2)" - # - # sanitize_sql_for_order("id ASC") - # # => "id ASC" - def sanitize_sql_for_order(condition) # :doc: - if condition.is_a?(Array) && condition.first.to_s.include?("?") - enforce_raw_sql_whitelist([condition.first], - whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST - ) - - # Ensure we aren't dealing with a subclass of String that might - # override methods we use (eg. Arel::Nodes::SqlLiteral). - if condition.first.kind_of?(String) && !condition.first.instance_of?(String) - condition = [String.new(condition.first), *condition[1..-1]] - end - - Arel.sql(sanitize_sql_array(condition)) - else - condition + end + + # Accepts an array, or string of SQL conditions and sanitizes + # them into a valid SQL fragment for an ORDER clause. + # + # sanitize_sql_for_order(["field(id, ?)", [1,3,2]]) + # # => "field(id, 1,3,2)" + # + # sanitize_sql_for_order("id ASC") + # # => "id ASC" + def sanitize_sql_for_order(condition) + if condition.is_a?(Array) && condition.first.to_s.include?("?") + enforce_raw_sql_whitelist([condition.first], + whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST + ) + + # Ensure we aren't dealing with a subclass of String that might + # override methods we use (eg. Arel::Nodes::SqlLiteral). + if condition.first.kind_of?(String) && !condition.first.instance_of?(String) + condition = [String.new(condition.first), *condition[1..-1]] end + + Arel.sql(sanitize_sql_array(condition)) + else + condition end + end + + # Sanitizes a hash of attribute/value pairs into SQL conditions for a SET clause. + # + # sanitize_sql_hash_for_assignment({ status: nil, group_id: 1 }, "posts") + # # => "`posts`.`status` = NULL, `posts`.`group_id` = 1" + def sanitize_sql_hash_for_assignment(attrs, table) + c = connection + attrs.map do |attr, value| + type = type_for_attribute(attr.to_s) + value = type.serialize(type.cast(value)) + "#{c.quote_table_name_for_assignment(table, attr)} = #{c.quote(value)}" + end.join(", ") + end + + # Sanitizes a +string+ so that it is safe to use within an SQL + # LIKE statement. This method uses +escape_character+ to escape all occurrences of "\", "_" and "%". + # + # sanitize_sql_like("100%") + # # => "100\\%" + # + # sanitize_sql_like("snake_cased_string") + # # => "snake\\_cased\\_string" + # + # sanitize_sql_like("100%", "!") + # # => "100!%" + # + # sanitize_sql_like("snake_cased_string", "!") + # # => "snake!_cased!_string" + def sanitize_sql_like(string, escape_character = "\\") + pattern = Regexp.union(escape_character, "%", "_") + string.gsub(pattern) { |x| [escape_character, x].join } + end + + # Accepts an array of conditions. The array has each value + # sanitized and interpolated into the SQL statement. + # + # sanitize_sql_array(["name=? and group_id=?", "foo'bar", 4]) + # # => "name='foo''bar' and group_id=4" + # + # sanitize_sql_array(["name=:name and group_id=:group_id", name: "foo'bar", group_id: 4]) + # # => "name='foo''bar' and group_id=4" + # + # sanitize_sql_array(["name='%s' and group_id='%s'", "foo'bar", 4]) + # # => "name='foo''bar' and group_id='4'" + def sanitize_sql_array(ary) + statement, *values = ary + if values.first.is_a?(Hash) && /:\w+/.match?(statement) + replace_named_bind_variables(statement, values.first) + elsif statement.include?("?") + replace_bind_variables(statement, values) + elsif statement.blank? + statement + else + statement % values.collect { |value| connection.quote_string(value.to_s) } + end + end + private # Accepts a hash of SQL conditions and replaces those attributes # that correspond to a {#composed_of}[rdoc-ref:Aggregations::ClassMethods#composed_of] # relationship with their expanded aggregate attribute values. @@ -113,62 +168,6 @@ module ActiveRecord expanded_attrs end - # Sanitizes a hash of attribute/value pairs into SQL conditions for a SET clause. - # - # sanitize_sql_hash_for_assignment({ status: nil, group_id: 1 }, "posts") - # # => "`posts`.`status` = NULL, `posts`.`group_id` = 1" - def sanitize_sql_hash_for_assignment(attrs, table) # :doc: - c = connection - attrs.map do |attr, value| - type = type_for_attribute(attr.to_s) - value = type.serialize(type.cast(value)) - "#{c.quote_table_name_for_assignment(table, attr)} = #{c.quote(value)}" - end.join(", ") - end - - # Sanitizes a +string+ so that it is safe to use within an SQL - # LIKE statement. This method uses +escape_character+ to escape all occurrences of "\", "_" and "%". - # - # sanitize_sql_like("100%") - # # => "100\\%" - # - # sanitize_sql_like("snake_cased_string") - # # => "snake\\_cased\\_string" - # - # sanitize_sql_like("100%", "!") - # # => "100!%" - # - # sanitize_sql_like("snake_cased_string", "!") - # # => "snake!_cased!_string" - def sanitize_sql_like(string, escape_character = "\\") # :doc: - pattern = Regexp.union(escape_character, "%", "_") - string.gsub(pattern) { |x| [escape_character, x].join } - end - - # Accepts an array of conditions. The array has each value - # sanitized and interpolated into the SQL statement. - # - # sanitize_sql_array(["name=? and group_id=?", "foo'bar", 4]) - # # => "name='foo''bar' and group_id=4" - # - # sanitize_sql_array(["name=:name and group_id=:group_id", name: "foo'bar", group_id: 4]) - # # => "name='foo''bar' and group_id=4" - # - # sanitize_sql_array(["name='%s' and group_id='%s'", "foo'bar", 4]) - # # => "name='foo''bar' and group_id='4'" - def sanitize_sql_array(ary) # :doc: - statement, *values = ary - if values.first.is_a?(Hash) && /:\w+/.match?(statement) - replace_named_bind_variables(statement, values.first) - elsif statement.include?("?") - replace_bind_variables(statement, values) - elsif statement.blank? - statement - else - statement % values.collect { |value| connection.quote_string(value.to_s) } - end - end - def replace_bind_variables(statement, values) raise_if_bind_arity_mismatch(statement, statement.count("?"), values.size) bound = values.dup diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 66f7d29886..16ccba6b6c 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -184,8 +184,9 @@ HEADER "name: #{index.name.inspect}", ] index_parts << "unique: true" if index.unique - index_parts << "length: { #{format_options(index.lengths)} }" if index.lengths.present? - index_parts << "order: { #{format_options(index.orders)} }" if index.orders.present? + index_parts << "length: #{format_index_parts(index.lengths)}" if index.lengths.present? + index_parts << "order: #{format_index_parts(index.orders)}" if index.orders.present? + index_parts << "opclass: #{format_index_parts(index.opclasses)}" if index.opclasses.present? index_parts << "where: #{index.where.inspect}" if index.where index_parts << "using: #{index.using.inspect}" if !@connection.default_index_type?(index) index_parts << "type: #{index.type.inspect}" if index.type @@ -231,6 +232,14 @@ HEADER options.map { |key, value| "#{key}: #{value.inspect}" }.join(", ") end + def format_index_parts(options) + if options.is_a?(Hash) + "{ #{format_options(options)} }" + else + options.inspect + end + end + def remove_prefix_and_suffix(table) prefix = Regexp.escape(@options[:table_name_prefix].to_s) suffix = Regexp.escape(@options[:table_name_suffix].to_s) diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb index e61c70848a..13b4096671 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -174,10 +174,10 @@ class Mysql2ConnectionTest < ActiveRecord::Mysql2TestCase assert_equal "SCHEMA", @subscriber.logged[0][1] end - def test_logs_name_rename_column_sql + def test_logs_name_rename_column_for_alter @connection.execute "CREATE TABLE `bar_baz` (`foo` varchar(255))" @subscriber.logged.clear - @connection.send(:rename_column_sql, "bar_baz", "foo", "foo2") + @connection.send(:rename_column_for_alter, "bar_baz", "foo", "foo2") assert_equal "SCHEMA", @subscriber.logged[0][1] ensure @connection.execute "DROP TABLE `bar_baz`" diff --git a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb index 9929237546..99c53dadeb 100644 --- a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb @@ -59,6 +59,9 @@ class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase assert_equal expected, add_index(:people, "lower(last_name)", using: type, unique: true) end + expected = %(CREATE INDEX "index_people_on_last_name" ON "people" USING gist ("last_name" bpchar_pattern_ops)) + assert_equal expected, add_index(:people, :last_name, using: :gist, opclass: { last_name: :bpchar_pattern_ops }) + assert_raise ArgumentError do add_index(:people, :last_name, algorithm: :copy) end diff --git a/activerecord/test/cases/adapters/postgresql/domain_test.rb b/activerecord/test/cases/adapters/postgresql/domain_test.rb index 9c3817e2ad..dafbc0a3db 100644 --- a/activerecord/test/cases/adapters/postgresql/domain_test.rb +++ b/activerecord/test/cases/adapters/postgresql/domain_test.rb @@ -44,6 +44,6 @@ class PostgresqlDomainTest < ActiveRecord::PostgreSQLTestCase record.price = "34.15" record.save! - assert_equal BigDecimal.new("34.15"), record.reload.price + assert_equal BigDecimal("34.15"), record.reload.price end end diff --git a/activerecord/test/cases/adapters/postgresql/money_test.rb b/activerecord/test/cases/adapters/postgresql/money_test.rb index 563f0bbfae..cc10890fa8 100644 --- a/activerecord/test/cases/adapters/postgresql/money_test.rb +++ b/activerecord/test/cases/adapters/postgresql/money_test.rb @@ -33,8 +33,8 @@ class PostgresqlMoneyTest < ActiveRecord::PostgreSQLTestCase end def test_default - assert_equal BigDecimal.new("150.55"), PostgresqlMoney.column_defaults["depth"] - assert_equal BigDecimal.new("150.55"), PostgresqlMoney.new.depth + assert_equal BigDecimal("150.55"), PostgresqlMoney.column_defaults["depth"] + assert_equal BigDecimal("150.55"), PostgresqlMoney.new.depth end def test_money_values @@ -65,7 +65,7 @@ class PostgresqlMoneyTest < ActiveRecord::PostgreSQLTestCase money = PostgresqlMoney.create(wealth: "987.65".dup) assert_equal 987.65, money.wealth - new_value = BigDecimal.new("123.45") + new_value = BigDecimal("123.45") money.wealth = new_value money.save! money.reload diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index f199519d86..1951230c8a 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -248,12 +248,12 @@ module ActiveRecord def test_index_with_opclass with_example_table do - @connection.add_index "ex", "data varchar_pattern_ops" - index = @connection.indexes("ex").find { |idx| idx.name == "index_ex_on_data_varchar_pattern_ops" } - assert_equal "data varchar_pattern_ops", index.columns + @connection.add_index "ex", "data", opclass: "varchar_pattern_ops" + index = @connection.indexes("ex").find { |idx| idx.name == "index_ex_on_data" } + assert_equal ["data"], index.columns - @connection.remove_index "ex", "data varchar_pattern_ops" - assert_not @connection.indexes("ex").find { |idx| idx.name == "index_ex_on_data_varchar_pattern_ops" } + @connection.remove_index "ex", "data" + assert_not @connection.indexes("ex").find { |idx| idx.name == "index_ex_on_data" } end end diff --git a/activerecord/test/cases/adapters/postgresql/range_test.rb b/activerecord/test/cases/adapters/postgresql/range_test.rb index a75fdef698..813a8721a2 100644 --- a/activerecord/test/cases/adapters/postgresql/range_test.rb +++ b/activerecord/test/cases/adapters/postgresql/range_test.rb @@ -134,10 +134,10 @@ _SQL end def test_numrange_values - assert_equal BigDecimal.new("0.1")..BigDecimal.new("0.2"), @first_range.num_range - assert_equal BigDecimal.new("0.1")...BigDecimal.new("0.2"), @second_range.num_range - assert_equal BigDecimal.new("0.1")...BigDecimal.new("Infinity"), @third_range.num_range - assert_equal BigDecimal.new("-Infinity")...BigDecimal.new("Infinity"), @fourth_range.num_range + assert_equal BigDecimal("0.1")..BigDecimal("0.2"), @first_range.num_range + assert_equal BigDecimal("0.1")...BigDecimal("0.2"), @second_range.num_range + assert_equal BigDecimal("0.1")...BigDecimal("Infinity"), @third_range.num_range + assert_equal BigDecimal("-Infinity")...BigDecimal("Infinity"), @fourth_range.num_range assert_nil @empty_range.num_range end @@ -285,14 +285,14 @@ _SQL def test_create_numrange assert_equal_round_trip(@new_range, :num_range, - BigDecimal.new("0.5")...BigDecimal.new("1")) + BigDecimal("0.5")...BigDecimal("1")) end def test_update_numrange assert_equal_round_trip(@first_range, :num_range, - BigDecimal.new("0.5")...BigDecimal.new("1")) + BigDecimal("0.5")...BigDecimal("1")) assert_nil_round_trip(@first_range, :num_range, - BigDecimal.new("0.5")...BigDecimal.new("0.5")) + BigDecimal("0.5")...BigDecimal("0.5")) end def test_create_daterange diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index 5a64da028b..2c99fa78bd 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -459,7 +459,7 @@ class SchemaTest < ActiveRecord::PostgreSQLTestCase assert_equal :btree, index_d.using assert_equal :gin, index_e.using - assert_equal :desc, index_d.orders[INDEX_D_COLUMN] + assert_equal :desc, index_d.orders end end @@ -500,6 +500,38 @@ class SchemaForeignKeyTest < ActiveRecord::PostgreSQLTestCase end end +class SchemaIndexOpclassTest < ActiveRecord::PostgreSQLTestCase + include SchemaDumpingHelper + + setup do + @connection = ActiveRecord::Base.connection + @connection.create_table "trains" do |t| + t.string :name + t.text :description + end + end + + teardown do + @connection.drop_table "trains", if_exists: true + end + + def test_string_opclass_is_dumped + @connection.execute "CREATE INDEX trains_name_and_description ON trains USING btree(name text_pattern_ops, description text_pattern_ops)" + + output = dump_table_schema "trains" + + assert_match(/opclass: :text_pattern_ops/, output) + end + + def test_non_default_opclass_is_dumped + @connection.execute "CREATE INDEX trains_name_and_description ON trains USING btree(name, description text_pattern_ops)" + + output = dump_table_schema "trains" + + assert_match(/opclass: \{ description: :text_pattern_ops \}/, output) + end +end + class DefaultsUsingMultipleSchemasAndDomainTest < ActiveRecord::PostgreSQLTestCase setup do @connection = ActiveRecord::Base.connection @@ -534,7 +566,7 @@ class DefaultsUsingMultipleSchemasAndDomainTest < ActiveRecord::PostgreSQLTestCa end def test_decimal_defaults_in_new_schema_when_overriding_domain - assert_equal BigDecimal.new("3.14159265358979323846"), Default.new.decimal_col, "Default of decimal column was not correctly parsed" + assert_equal BigDecimal("3.14159265358979323846"), Default.new.decimal_col, "Default of decimal column was not correctly parsed" end def test_bpchar_defaults_in_new_schema_when_overriding_domain diff --git a/activerecord/test/cases/adapters/postgresql/transaction_test.rb b/activerecord/test/cases/adapters/postgresql/transaction_test.rb index c24dfeb345..9821b103df 100644 --- a/activerecord/test/cases/adapters/postgresql/transaction_test.rb +++ b/activerecord/test/cases/adapters/postgresql/transaction_test.rb @@ -14,6 +14,7 @@ module ActiveRecord setup do @abort, Thread.abort_on_exception = Thread.abort_on_exception, false + Thread.report_on_exception, @original_report_on_exception = false, Thread.report_on_exception if Thread.respond_to?(:report_on_exception) @connection = ActiveRecord::Base.connection @@ -31,6 +32,7 @@ module ActiveRecord @connection.drop_table "samples", if_exists: true Thread.abort_on_exception = @abort + Thread.report_on_exception = @original_report_on_exception if Thread.respond_to?(:report_on_exception) end test "raises SerializationFailure when a serialization failure occurs" do diff --git a/activerecord/test/cases/adapters/sqlite3/json_test.rb b/activerecord/test/cases/adapters/sqlite3/json_test.rb index 568a524058..6f247fcd22 100644 --- a/activerecord/test/cases/adapters/sqlite3/json_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/json_test.rb @@ -9,8 +9,8 @@ class SQLite3JSONTest < ActiveRecord::SQLite3TestCase def setup super @connection.create_table("json_data_type") do |t| - t.column "payload", :json, default: {} - t.column "settings", :json + t.json "payload", default: {} + t.json "settings" end end diff --git a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb index de422fad23..6fdb353368 100644 --- a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb @@ -38,7 +38,7 @@ class SQLite3QuotingTest < ActiveRecord::SQLite3TestCase end def test_type_cast_bigdecimal - bd = BigDecimal.new "10.0" + bd = BigDecimal "10.0" assert_equal bd.to_f, @conn.type_cast(bd) end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 1f057fe5c6..1357719422 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -360,6 +360,51 @@ module ActiveRecord end end + class Barcode < ActiveRecord::Base + self.primary_key = "code" + end + + def test_copy_table_with_existing_records_have_custom_primary_key + connection = Barcode.connection + connection.create_table(:barcodes, primary_key: "code", id: :string, limit: 42, force: true) do |t| + t.text :other_attr + end + code = "214fe0c2-dd47-46df-b53b-66090b3c1d40" + Barcode.create!(code: code, other_attr: "xxx") + + connection.change_table "barcodes" do |t| + connection.remove_column("barcodes", "other_attr") + end + + assert_equal code, Barcode.first.id + ensure + Barcode.reset_column_information + end + + def test_copy_table_with_composite_primary_keys + connection = Barcode.connection + connection.create_table(:barcodes, primary_key: ["region", "code"], force: true) do |t| + t.string :region + t.string :code + t.text :other_attr + end + region = "US" + code = "214fe0c2-dd47-46df-b53b-66090b3c1d40" + Barcode.create!(region: region, code: code, other_attr: "xxx") + + connection.change_table "barcodes" do |t| + connection.remove_column("barcodes", "other_attr") + end + + assert_equal ["region", "code"], connection.primary_keys("barcodes") + + barcode = Barcode.first + assert_equal region, barcode.region + assert_equal code, barcode.code + ensure + Barcode.reset_column_information + end + def test_supports_extensions assert_not @conn.supports_extensions?, "does not support extensions" end diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 829e12fbc8..e717621928 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -136,7 +136,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_multiple_stis_and_order - author = Author.all.merge!(includes: { posts: [ :special_comments , :very_special_comment ] }, order: ["authors.name", "comments.body", "very_special_comments_posts.body"], where: "posts.id = 4").first + author = Author.all.merge!(includes: { posts: [ :special_comments, :very_special_comment ] }, order: ["authors.name", "comments.body", "very_special_comments_posts.body"], where: "posts.id = 4").first assert_equal authors(:david), author assert_no_queries do author.posts.first.special_comments diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 9a042c74db..2649dc010f 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1073,7 +1073,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_load_with_sti_sharing_association - assert_queries(2) do #should not do 1 query per subclass + assert_queries(2) do # should not do 1 query per subclass Comment.includes(:post).to_a end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 4ca11af791..5ed8d0ee81 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2350,7 +2350,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase test "does not duplicate associations when used with natural primary keys" do speedometer = Speedometer.create!(id: "4") - speedometer.minivans.create!(minivan_id: "a-van-red" , name: "a van", color: "red") + speedometer.minivans.create!(minivan_id: "a-van-red", name: "a van", color: "red") assert_equal 1, speedometer.minivans.to_a.size, "Only one association should be present:\n#{speedometer.minivans.to_a}" assert_equal 1, speedometer.reload.minivans.to_a.size diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index fe24c465b2..1d37457464 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -100,7 +100,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase end def test_has_one_through_eager_loading - members = assert_queries(3) do #base table, through table, clubs table + members = assert_queries(3) do # base table, through table, clubs table Member.all.merge!(includes: :club, where: ["name = ?", "Groucho Marx"]).to_a end assert_equal 1, members.size @@ -108,7 +108,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase end def test_has_one_through_eager_loading_through_polymorphic - members = assert_queries(3) do #base table, through table, clubs table + members = assert_queries(3) do # base table, through table, clubs table Member.all.merge!(includes: :sponsor_club, where: ["name = ?", "Groucho Marx"]).to_a end assert_equal 1, members.size @@ -139,7 +139,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_has_one_through_nonpreload_eagerloading members = assert_queries(1) do - Member.all.merge!(includes: :club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name").to_a #force fallback + Member.all.merge!(includes: :club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name").to_a # force fallback end assert_equal 1, members.size assert_not_nil assert_no_queries { members[0].club } @@ -147,7 +147,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_has_one_through_nonpreload_eager_loading_through_polymorphic members = assert_queries(1) do - Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name").to_a #force fallback + Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name").to_a # force fallback end assert_equal 1, members.size assert_not_nil assert_no_queries { members[0].sponsor_club } @@ -156,7 +156,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_has_one_through_nonpreload_eager_loading_through_polymorphic_with_more_than_one_through_record Sponsor.new(sponsor_club: clubs(:crazy_club), sponsorable: members(:groucho)).save! members = assert_queries(1) do - Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name DESC").to_a #force fallback + Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name DESC").to_a # force fallback end assert_equal 1, members.size assert_not_nil assert_no_queries { members[0].sponsor_club } diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 87694b0788..5d83c9435b 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -467,7 +467,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase new_tag = Tag.new(name: "new") saved_post.tags << new_tag - assert new_tag.persisted? #consistent with habtm! + assert new_tag.persisted? # consistent with habtm! assert saved_post.persisted? assert_includes saved_post.tags, new_tag diff --git a/activerecord/test/cases/attributes_test.rb b/activerecord/test/cases/attributes_test.rb index 2caf2a63d4..8ebfee61ff 100644 --- a/activerecord/test/cases/attributes_test.rb +++ b/activerecord/test/cases/attributes_test.rb @@ -59,7 +59,7 @@ module ActiveRecord test "nonexistent attribute" do data = OverloadedType.new(non_existent_decimal: 1) - assert_equal BigDecimal.new(1), data.non_existent_decimal + assert_equal BigDecimal(1), data.non_existent_decimal assert_raise ActiveRecord::UnknownAttributeError do UnoverloadedType.new(non_existent_decimal: 1) end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index d79afa2ee9..7fc4a396e4 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -629,7 +629,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_readonly_attributes - assert_equal Set.new([ "title" , "comments_count" ]), ReadonlyTitlePost.readonly_attributes + assert_equal Set.new([ "title", "comments_count" ]), ReadonlyTitlePost.readonly_attributes post = ReadonlyTitlePost.create(title: "cannot change this", body: "changeable") post.reload @@ -891,11 +891,9 @@ class BasicsTest < ActiveRecord::TestCase assert_equal 2147483648, company.rating end - unless current_adapter?(:SQLite3Adapter) - def test_bignum_pk - company = Company.create!(id: 2147483648, name: "foo") - assert_equal company, Company.find(company.id) - end + def test_bignum_pk + company = Company.create!(id: 2147483648, name: "foo") + assert_equal company, Company.find(company.id) end # TODO: extend defaults tests to other databases! @@ -1497,4 +1495,24 @@ class BasicsTest < ActiveRecord::TestCase # regular column assert query.include?("name") end + + test "column names are quoted when using #from clause and model has ignored columns" do + refute_empty Developer.ignored_columns + query = Developer.from("`developers`").to_sql + quoted_id = Developer.connection.quote_table_name("id") + + assert_match(/SELECT #{quoted_id}.* FROM `developers`/, query) + end + + test "protected environments by default is an array with production" do + assert_equal ["production"], ActiveRecord::Base.protected_environments + end + + def test_protected_environments_are_stored_as_an_array_of_string + previous_protected_environments = ActiveRecord::Base.protected_environments + ActiveRecord::Base.protected_environments = [:staging, "production"] + assert_equal ["staging", "production"], ActiveRecord::Base.protected_environments + ensure + ActiveRecord::Base.protected_environments = previous_protected_environments + end end diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 1e08cc74dc..70c0ffb3bf 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -469,6 +469,7 @@ module ActiveRecord end def test_non_bang_disconnect_and_clear_reloadable_connections_throw_exception_if_threads_dont_return_their_conns + Thread.report_on_exception, original_report_on_exception = false, Thread.report_on_exception if Thread.respond_to?(:report_on_exception) @pool.checkout_timeout = 0.001 # no need to delay test suite by waiting the whole full default timeout [:disconnect, :clear_reloadable_connections].each do |group_action_method| @pool.with_connection do |connection| @@ -477,6 +478,8 @@ module ActiveRecord end end end + ensure + Thread.report_on_exception = original_report_on_exception if Thread.respond_to?(:report_on_exception) end def test_disconnect_and_clear_reloadable_connections_attempt_to_wait_for_threads_to_return_their_conns diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index 4690682cd8..3d11b573f1 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -53,7 +53,7 @@ class DefaultNumbersTest < ActiveRecord::TestCase def test_default_decimal_number record = DefaultNumber.new - assert_equal BigDecimal.new("2.78"), record.decimal_number + assert_equal BigDecimal("2.78"), record.decimal_number assert_equal "2.78", record.decimal_number_before_type_cast end end diff --git a/activerecord/test/cases/dup_test.rb b/activerecord/test/cases/dup_test.rb index 2fefdbf204..73da31996e 100644 --- a/activerecord/test/cases/dup_test.rb +++ b/activerecord/test/cases/dup_test.rb @@ -62,10 +62,10 @@ module ActiveRecord topic.attributes = dbtopic.attributes.except("id") - #duped has no timestamp values + # duped has no timestamp values duped = dbtopic.dup - #clear topic timestamp values + # clear topic timestamp values topic.send(:clear_timestamp_attributes) assert_equal topic.changes, duped.changes @@ -100,7 +100,7 @@ module ActiveRecord # temporary change to the topic object topic.updated_at -= 3.days - #dup should not preserve the timestamps if present + # dup should not preserve the timestamps if present new_topic = topic.dup assert_nil new_topic.updated_at assert_nil new_topic.created_at diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index e936c56ab8..62d5d88fcc 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -565,7 +565,7 @@ class FinderTest < ActiveRecord::TestCase assert_nil Topic.offset(4).second_to_last assert_nil Topic.offset(5).second_to_last - #test with limit + # test with limit assert_nil Topic.limit(1).second assert_nil Topic.limit(1).second_to_last end @@ -1238,7 +1238,7 @@ class FinderTest < ActiveRecord::TestCase test "find_by with associations" do assert_equal authors(:david), Post.find_by(author: authors(:david)).author - assert_equal authors(:mary) , Post.find_by(author: authors(:mary)).author + assert_equal authors(:mary), Post.find_by(author: authors(:mary)).author end test "find_by doesn't have implicit ordering" do diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index b0b63f5203..8e8a49af8e 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -247,7 +247,7 @@ class FixturesTest < ActiveRecord::TestCase def test_nonexistent_fixture_file nonexistent_fixture_path = FIXTURES_ROOT + "/imnothere" - #sanity check to make sure that this file never exists + # sanity check to make sure that this file never exists assert Dir[nonexistent_fixture_path + "*"].empty? assert_raise(Errno::ENOENT) do diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index c931f7d21c..ff4385c8b4 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -280,6 +280,21 @@ class InheritanceTest < ActiveRecord::TestCase assert_equal Firm, firm.class end + def test_where_new_with_subclass + firm = Company.where(type: "Firm").new + assert_equal Firm, firm.class + end + + def test_where_create_with_subclass + firm = Company.where(type: "Firm").create(name: "Basecamp") + assert_equal Firm, firm.class + end + + def test_where_create_bang_with_subclass + firm = Company.where(type: "Firm").create!(name: "Basecamp") + assert_equal Firm, firm.class + end + def test_new_with_abstract_class e = assert_raises(NotImplementedError) do AbstractCompany.new @@ -302,6 +317,30 @@ class InheritanceTest < ActiveRecord::TestCase assert_raise(ActiveRecord::SubclassNotFound) { Company.new(type: "Account") } end + def test_where_new_with_invalid_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "InvalidType").new } + end + + def test_where_new_with_unrelated_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "Account").new } + end + + def test_where_create_with_invalid_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "InvalidType").create } + end + + def test_where_create_with_unrelated_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "Account").create } + end + + def test_where_create_bang_with_invalid_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "InvalidType").create! } + end + + def test_where_create_bang_with_unrelated_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.where(type: "Account").create! } + end + def test_new_with_unrelated_namespaced_type without_store_full_sti_class do e = assert_raises ActiveRecord::SubclassNotFound do diff --git a/activerecord/test/cases/json_attribute_test.rb b/activerecord/test/cases/json_attribute_test.rb index 63f3c77fc3..afc39d0420 100644 --- a/activerecord/test/cases/json_attribute_test.rb +++ b/activerecord/test/cases/json_attribute_test.rb @@ -19,14 +19,14 @@ class JsonAttributeTest < ActiveRecord::TestCase def setup super @connection.create_table("json_data_type") do |t| - t.text "payload" - t.text "settings" + t.string "payload" + t.string "settings" end end private def column_type - :text + :string end def klass diff --git a/activerecord/test/cases/json_shared_test_cases.rb b/activerecord/test/cases/json_shared_test_cases.rb index a71485982c..b0c0f2c283 100644 --- a/activerecord/test/cases/json_shared_test_cases.rb +++ b/activerecord/test/cases/json_shared_test_cases.rb @@ -23,25 +23,23 @@ module JSONSharedTestCases def test_column column = klass.columns_hash["payload"] assert_equal column_type, column.type - assert_equal column_type.to_s, column.sql_type + assert_type_match column_type, column.sql_type type = klass.type_for_attribute("payload") assert_not type.binary? end def test_change_table_supports_json - skip unless @connection.supports_json? @connection.change_table("json_data_type") do |t| t.public_send column_type, "users" end klass.reset_column_information column = klass.columns_hash["users"] assert_equal column_type, column.type - assert_equal column_type.to_s, column.sql_type + assert_type_match column_type, column.sql_type end def test_schema_dumping - skip unless @connection.supports_json? output = dump_table_schema("json_data_type") assert_match(/t\.#{column_type}\s+"settings"/, output) end @@ -68,26 +66,26 @@ module JSONSharedTestCases end def test_rewrite - @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k":"v"}')|) + @connection.execute(insert_statement_per_database('{"k":"v"}')) x = klass.first x.payload = { '"a\'' => "b" } assert x.save! end def test_select - @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k":"v"}')|) + @connection.execute(insert_statement_per_database('{"k":"v"}')) x = klass.first assert_equal({ "k" => "v" }, x.payload) end def test_select_multikey - @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')|) + @connection.execute(insert_statement_per_database('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')) x = klass.first assert_equal({ "k1" => "v1", "k2" => "v2", "k3" => [1, 2, 3] }, x.payload) end def test_null_json - @connection.execute("insert into json_data_type (payload) VALUES(null)") + @connection.execute(insert_statement_per_database("null")) x = klass.first assert_nil(x.payload) end @@ -109,13 +107,13 @@ module JSONSharedTestCases end def test_select_array_json_value - @connection.execute(%q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')|) + @connection.execute(insert_statement_per_database('["v0",{"k1":"v1"}]')) x = klass.first assert_equal(["v0", { "k1" => "v1" }], x.payload) end def test_rewrite_array_json_value - @connection.execute(%q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')|) + @connection.execute(insert_statement_per_database('["v0",{"k1":"v1"}]')) x = klass.first x.payload = ["v1", { "k2" => "v2" }, "v3"] assert x.save! @@ -255,4 +253,17 @@ module JSONSharedTestCases def klass JsonDataType end + + def assert_type_match(type, sql_type) + native_type = ActiveRecord::Base.connection.native_database_types[type][:name] + assert_match %r(\A#{native_type}\b), sql_type + end + + def insert_statement_per_database(values) + if current_adapter?(:OracleAdapter) + "insert into json_data_type (id, payload) VALUES (json_data_type_seq.nextval, '#{values}')" + else + "insert into json_data_type (payload) VALUES ('#{values}')" + end + end end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index e857180bd1..437a5a38a3 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -399,11 +399,43 @@ class OptimisticLockingTest < ActiveRecord::TestCase end end + def test_counter_cache_with_touch_and_lock_version + car = Car.create! + + assert_equal 0, car.wheels_count + assert_equal 0, car.lock_version + + previously_car_updated_at = car.updated_at + travel(1.second) do + Wheel.create!(wheelable: car) + end + + assert_equal 1, car.reload.wheels_count + assert_not_equal previously_car_updated_at, car.updated_at + assert_equal 1, car.lock_version + + previously_car_updated_at = car.updated_at + car.wheels.first.update(size: 42) + + assert_equal 1, car.reload.wheels_count + assert_not_equal previously_car_updated_at, car.updated_at + assert_equal 2, car.lock_version + + previously_car_updated_at = car.updated_at + travel(1.second) do + car.wheels.first.destroy! + end + + assert_equal 0, car.reload.wheels_count + assert_not_equal previously_car_updated_at, car.updated_at + assert_equal 3, car.lock_version + end + def test_polymorphic_destroy_with_dependencies_and_lock_version car = Car.create! assert_difference "car.wheels.count" do - car.wheels << Wheel.create! + car.wheels.create end assert_difference "car.wheels.count", -1 do car.reload.destroy diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index 208e54ed0b..e2742ed33e 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -33,8 +33,9 @@ class LogSubscriberTest < ActiveRecord::TestCase super end - def debug(message) - @debugs << message + def debug(progname = nil, &block) + @debugs << progname + super end end @@ -171,6 +172,22 @@ class LogSubscriberTest < ActiveRecord::TestCase assert_match(/SELECT .*?FROM .?developers.?/i, @logger.logged(:debug).last) end + def test_vebose_query_logs + ActiveRecord::Base.verbose_query_logs = true + + logger = TestDebugLogSubscriber.new + logger.sql(Event.new(0, sql: "hi mom!")) + assert_match(/↳/, @logger.logged(:debug).last) + ensure + ActiveRecord::Base.verbose_query_logs = false + end + + def test_verbose_query_logs_disabled_by_default + logger = TestDebugLogSubscriber.new + logger.sql(Event.new(0, sql: "hi mom!")) + assert_no_match(/↳/, @logger.logged(:debug).last) + end + def test_cached_queries ActiveRecord::Base.cache do Developer.all.load diff --git a/activerecord/test/cases/migration/change_schema_test.rb b/activerecord/test/cases/migration/change_schema_test.rb index 7b0644e9c0..38a906c8f5 100644 --- a/activerecord/test/cases/migration/change_schema_test.rb +++ b/activerecord/test/cases/migration/change_schema_test.rb @@ -264,19 +264,18 @@ module ActiveRecord t.column :foo, :timestamp end - klass = Class.new(ActiveRecord::Base) - klass.table_name = "testings" + column = connection.columns(:testings).find { |c| c.name == "foo" } - assert_equal :datetime, klass.columns_hash["foo"].type + assert_equal :datetime, column.type if current_adapter?(:PostgreSQLAdapter) - assert_equal "timestamp without time zone", klass.columns_hash["foo"].sql_type + assert_equal "timestamp without time zone", column.sql_type elsif current_adapter?(:Mysql2Adapter) - assert_equal "timestamp", klass.columns_hash["foo"].sql_type + assert_equal "timestamp", column.sql_type elsif current_adapter?(:OracleAdapter) - assert_equal "TIMESTAMP(6)", klass.columns_hash["foo"].sql_type + assert_equal "TIMESTAMP(6)", column.sql_type else - assert_equal klass.connection.type_to_sql("datetime"), klass.columns_hash["foo"].sql_type + assert_equal connection.type_to_sql("datetime"), column.sql_type end end diff --git a/activerecord/test/cases/migration/column_attributes_test.rb b/activerecord/test/cases/migration/column_attributes_test.rb index be6dc2acb1..3022121f4c 100644 --- a/activerecord/test/cases/migration/column_attributes_test.rb +++ b/activerecord/test/cases/migration/column_attributes_test.rb @@ -80,7 +80,7 @@ module ActiveRecord TestModel.delete_all # Now use the Rails insertion - TestModel.create wealth: BigDecimal.new("12345678901234567890.0123456789") + TestModel.create wealth: BigDecimal("12345678901234567890.0123456789") # SELECT row = TestModel.first @@ -146,7 +146,7 @@ module ActiveRecord TestModel.create first_name: "bob", last_name: "bobsen", bio: "I was born ....", age: 18, height: 1.78, - wealth: BigDecimal.new("12345678901234567890.0123456789"), + wealth: BigDecimal("12345678901234567890.0123456789"), birthday: 18.years.ago, favorite_day: 10.days.ago, moment_of_truth: "1782-10-10 21:40:18", male: true @@ -159,7 +159,7 @@ module ActiveRecord # Test for 30 significant digits (beyond the 16 of float), 10 of them # after the decimal place. - assert_equal BigDecimal.new("0012345678901234567890.0123456789"), bob.wealth + assert_equal BigDecimal("0012345678901234567890.0123456789"), bob.wealth assert_equal true, bob.male? diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 2fef2f796e..cc2391f349 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -16,7 +16,7 @@ module ActiveRecord ActiveRecord::Migration.verbose = false connection.create_table :testings do |t| - t.column :foo, :string, limit: 100 + t.column :foo, :string, limit: 5 t.column :bar, :string, limit: 100 end end @@ -126,6 +126,25 @@ module ActiveRecord end assert_match(/LegacyMigration < ActiveRecord::Migration\[4\.2\]/, e.message) end + + if current_adapter?(:PostgreSQLAdapter) + class Testing < ActiveRecord::Base + end + + def test_legacy_change_column_with_null_executes_update + migration = Class.new(ActiveRecord::Migration[5.1]) { + def migrate(x) + change_column :testings, :foo, :string, limit: 10, null: false, default: "foobar" + end + }.new + + Testing.create! + ActiveRecord::Migrator.new(:up, [migration]).migrate + assert_equal ["foobar"], Testing.all.map(&:foo) + ensure + ActiveRecord::Base.clear_cache! + end + end end end end diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb index 499d072de5..079be04946 100644 --- a/activerecord/test/cases/migration/foreign_key_test.rb +++ b/activerecord/test/cases/migration/foreign_key_test.rb @@ -227,6 +227,74 @@ if ActiveRecord::Base.connection.supports_foreign_keys? end end + if ActiveRecord::Base.connection.supports_validate_constraints? + def test_add_invalid_foreign_key + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", validate: false + + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + + fk = foreign_keys.first + refute fk.validated? + end + + def test_validate_foreign_key_infers_column + @connection.add_foreign_key :astronauts, :rockets, validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, :rockets + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_key_by_column + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, column: "rocket_id" + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_key_by_symbol_column + @connection.add_foreign_key :astronauts, :rockets, column: :rocket_id, validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, column: :rocket_id + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_key_by_name + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", name: "fancy_named_fk", validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, name: "fancy_named_fk" + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_non_existing_foreign_key_raises + assert_raises ArgumentError do + @connection.validate_foreign_key :astronauts, :rockets + end + end + + def test_validate_constraint_by_name + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", name: "fancy_named_fk", validate: false + + @connection.validate_constraint :astronauts, "fancy_named_fk" + assert @connection.foreign_keys("astronauts").first.validated? + end + else + # Foreign key should still be created, but should not be invalid + def test_add_invalid_foreign_key + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", validate: false + + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + + fk = foreign_keys.first + assert fk.validated? + end + end + def test_schema_dumping @connection.add_foreign_key :astronauts, :rockets output = dump_table_schema "astronauts" diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index f088c064f5..4cc66a2e49 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -70,7 +70,7 @@ class PersistenceTest < ActiveRecord::TestCase end def test_update_many - topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" }, nil => {} } + topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } } updated = Topic.update(topic_data.keys, topic_data.values) assert_equal [1, 2], updated.map(&:id) @@ -78,10 +78,33 @@ class PersistenceTest < ActiveRecord::TestCase assert_equal "2 updated", Topic.find(2).content end + def test_update_many_with_duplicated_ids + updated = Topic.update([1, 1, 2], [ + { "content" => "1 duplicated" }, { "content" => "1 updated" }, { "content" => "2 updated" } + ]) + + assert_equal [1, 1, 2], updated.map(&:id) + assert_equal "1 updated", Topic.find(1).content + assert_equal "2 updated", Topic.find(2).content + end + + def test_update_many_with_invalid_id + topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" }, 99999 => {} } + + assert_raise(ActiveRecord::RecordNotFound) do + Topic.update(topic_data.keys, topic_data.values) + end + + assert_not_equal "1 updated", Topic.find(1).content + assert_not_equal "2 updated", Topic.find(2).content + end + def test_class_level_update_is_affected_by_scoping topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } } - assert_equal [], Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) } + assert_raise(ActiveRecord::RecordNotFound) do + Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) } + end assert_not_equal "1 updated", Topic.find(1).content assert_not_equal "2 updated", Topic.find(2).content @@ -175,15 +198,25 @@ class PersistenceTest < ActiveRecord::TestCase end def test_destroy_many - clients = Client.all.merge!(order: "id").find([2, 3]) + clients = Client.find([2, 3]) assert_difference("Client.count", -2) do - destroyed = Client.destroy([2, 3, nil]).sort_by(&:id) + destroyed = Client.destroy([2, 3]) assert_equal clients, destroyed assert destroyed.all?(&:frozen?), "destroyed clients should be frozen" end end + def test_destroy_many_with_invalid_id + clients = Client.find([2, 3]) + + assert_raise(ActiveRecord::RecordNotFound) do + Client.destroy([2, 3, 99999]) + end + + assert_equal clients, Client.find([2, 3]) + end + def test_becomes assert_kind_of Reply, topics(:first).becomes(Reply) assert_equal "The First Topic", topics(:first).becomes(Reply).title @@ -473,10 +506,18 @@ class PersistenceTest < ActiveRecord::TestCase assert_raise(ActiveRecord::RecordNotFound) { Topic.find(topic.id) } end - def test_record_not_found_exception + def test_find_raises_record_not_found_exception assert_raise(ActiveRecord::RecordNotFound) { Topic.find(99999) } end + def test_update_raises_record_not_found_exception + assert_raise(ActiveRecord::RecordNotFound) { Topic.update(99999, approved: true) } + end + + def test_destroy_raises_record_not_found_exception + assert_raise(ActiveRecord::RecordNotFound) { Topic.destroy(99999) } + end + def test_update_all assert_equal Topic.count, Topic.update_all("content = 'bulk updated!'") assert_equal "bulk updated!", Topic.find(1).content @@ -938,7 +979,9 @@ class PersistenceTest < ActiveRecord::TestCase should_not_be_destroyed_reply = Reply.create("title" => "hello", "content" => "world") Topic.find(1).replies << should_not_be_destroyed_reply - assert_nil Topic.where("1=0").scoping { Topic.destroy(1) } + assert_raise(ActiveRecord::RecordNotFound) do + Topic.where("1=0").scoping { Topic.destroy(1) } + end assert_nothing_raised { Topic.find(1) } assert_nothing_raised { Reply.find(should_not_be_destroyed_reply.id) } diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 7f6c2382ca..80016fc19d 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -298,6 +298,8 @@ class PrimaryKeyAnyTypeTest < ActiveRecord::TestCase assert_not column.null assert_equal :string, column.type assert_equal 42, column.limit + ensure + Barcode.reset_column_information end test "schema dump primary key includes type and options" do diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index 897d252cf8..6534770c57 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -111,7 +111,7 @@ module ActiveRecord end def test_quote_bigdecimal - bigdec = BigDecimal.new((1 << 100).to_s) + bigdec = BigDecimal((1 << 100).to_s) assert_equal bigdec.to_s("F"), @quoter.quote(bigdec) end diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index d95a54a2fe..99797528b2 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -265,7 +265,7 @@ module ActiveRecord end def test_where_with_decimal_for_string_column - count = Post.where(title: BigDecimal.new(0)).count + count = Post.where(title: BigDecimal(0)).count assert_equal 0, count end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index a71d8de521..b424ca91de 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -68,7 +68,7 @@ module ActiveRecord relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) left = relation.table[:id].eq(10) right = relation.table[:id].eq(10) - combine = left.and right + combine = left.or(right) relation.where! combine assert_equal({}, relation.where_values_hash) end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 50ad1d5b26..675aafabda 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1189,6 +1189,15 @@ class RelationTest < ActiveRecord::TestCase assert_equal "hen", hen.name end + def test_create_with_polymorphic_association + author = authors(:david) + post = posts(:welcome) + comment = Comment.where(post: post, author: author).create!(body: "hello") + + assert_equal author, comment.author + assert_equal post, comment.post + end + def test_first_or_create parrot = Bird.where(color: "green").first_or_create(name: "parrot") assert_kind_of Bird, parrot diff --git a/activerecord/test/cases/sanitize_test.rb b/activerecord/test/cases/sanitize_test.rb index 85a555fe35..86e45b3cbd 100644 --- a/activerecord/test/cases/sanitize_test.rb +++ b/activerecord/test/cases/sanitize_test.rb @@ -11,30 +11,30 @@ class SanitizeTest < ActiveRecord::TestCase def test_sanitize_sql_array_handles_string_interpolation quoted_bambi = ActiveRecord::Base.connection.quote_string("Bambi") - assert_equal "name='#{quoted_bambi}'", Binary.send(:sanitize_sql_array, ["name='%s'", "Bambi"]) - assert_equal "name='#{quoted_bambi}'", Binary.send(:sanitize_sql_array, ["name='%s'", "Bambi".mb_chars]) + assert_equal "name='#{quoted_bambi}'", Binary.sanitize_sql_array(["name='%s'", "Bambi"]) + assert_equal "name='#{quoted_bambi}'", Binary.sanitize_sql_array(["name='%s'", "Bambi".mb_chars]) quoted_bambi_and_thumper = ActiveRecord::Base.connection.quote_string("Bambi\nand\nThumper") - assert_equal "name='#{quoted_bambi_and_thumper}'", Binary.send(:sanitize_sql_array, ["name='%s'", "Bambi\nand\nThumper"]) - assert_equal "name='#{quoted_bambi_and_thumper}'", Binary.send(:sanitize_sql_array, ["name='%s'", "Bambi\nand\nThumper".mb_chars]) + assert_equal "name='#{quoted_bambi_and_thumper}'", Binary.sanitize_sql_array(["name='%s'", "Bambi\nand\nThumper"]) + assert_equal "name='#{quoted_bambi_and_thumper}'", Binary.sanitize_sql_array(["name='%s'", "Bambi\nand\nThumper".mb_chars]) end def test_sanitize_sql_array_handles_bind_variables quoted_bambi = ActiveRecord::Base.connection.quote("Bambi") - assert_equal "name=#{quoted_bambi}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi"]) - assert_equal "name=#{quoted_bambi}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi".mb_chars]) + assert_equal "name=#{quoted_bambi}", Binary.sanitize_sql_array(["name=?", "Bambi"]) + assert_equal "name=#{quoted_bambi}", Binary.sanitize_sql_array(["name=?", "Bambi".mb_chars]) quoted_bambi_and_thumper = ActiveRecord::Base.connection.quote("Bambi\nand\nThumper") - assert_equal "name=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi\nand\nThumper"]) - assert_equal "name=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi\nand\nThumper".mb_chars]) + assert_equal "name=#{quoted_bambi_and_thumper}", Binary.sanitize_sql_array(["name=?", "Bambi\nand\nThumper"]) + assert_equal "name=#{quoted_bambi_and_thumper}", Binary.sanitize_sql_array(["name=?", "Bambi\nand\nThumper".mb_chars]) end def test_sanitize_sql_array_handles_named_bind_variables quoted_bambi = ActiveRecord::Base.connection.quote("Bambi") - assert_equal "name=#{quoted_bambi}", Binary.send(:sanitize_sql_array, ["name=:name", name: "Bambi"]) - assert_equal "name=#{quoted_bambi} AND id=1", Binary.send(:sanitize_sql_array, ["name=:name AND id=:id", name: "Bambi", id: 1]) + assert_equal "name=#{quoted_bambi}", Binary.sanitize_sql_array(["name=:name", name: "Bambi"]) + assert_equal "name=#{quoted_bambi} AND id=1", Binary.sanitize_sql_array(["name=:name AND id=:id", name: "Bambi", id: 1]) quoted_bambi_and_thumper = ActiveRecord::Base.connection.quote("Bambi\nand\nThumper") - assert_equal "name=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=:name", name: "Bambi\nand\nThumper"]) - assert_equal "name=#{quoted_bambi_and_thumper} AND name2=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=:name AND name2=:name", name: "Bambi\nand\nThumper"]) + assert_equal "name=#{quoted_bambi_and_thumper}", Binary.sanitize_sql_array(["name=:name", name: "Bambi\nand\nThumper"]) + assert_equal "name=#{quoted_bambi_and_thumper} AND name2=#{quoted_bambi_and_thumper}", Binary.sanitize_sql_array(["name=:name AND name2=:name", name: "Bambi\nand\nThumper"]) end def test_sanitize_sql_array_handles_relations @@ -43,31 +43,31 @@ class SanitizeTest < ActiveRecord::TestCase sub_query_pattern = /\(\bselect\b.*?\bwhere\b.*?\)/i - select_author_sql = Post.send(:sanitize_sql_array, ["id in (?)", david_posts]) + select_author_sql = Post.sanitize_sql_array(["id in (?)", david_posts]) assert_match(sub_query_pattern, select_author_sql, "should sanitize `Relation` as subquery for bind variables") - select_author_sql = Post.send(:sanitize_sql_array, ["id in (:post_ids)", post_ids: david_posts]) + select_author_sql = Post.sanitize_sql_array(["id in (:post_ids)", post_ids: david_posts]) assert_match(sub_query_pattern, select_author_sql, "should sanitize `Relation` as subquery for named bind variables") end def test_sanitize_sql_array_handles_empty_statement - select_author_sql = Post.send(:sanitize_sql_array, [""]) + select_author_sql = Post.sanitize_sql_array([""]) assert_equal("", select_author_sql) end def test_sanitize_sql_like - assert_equal '100\%', Binary.send(:sanitize_sql_like, "100%") - assert_equal 'snake\_cased\_string', Binary.send(:sanitize_sql_like, "snake_cased_string") - assert_equal 'C:\\\\Programs\\\\MsPaint', Binary.send(:sanitize_sql_like, 'C:\\Programs\\MsPaint') - assert_equal "normal string 42", Binary.send(:sanitize_sql_like, "normal string 42") + assert_equal '100\%', Binary.sanitize_sql_like("100%") + assert_equal 'snake\_cased\_string', Binary.sanitize_sql_like("snake_cased_string") + assert_equal 'C:\\\\Programs\\\\MsPaint', Binary.sanitize_sql_like('C:\\Programs\\MsPaint') + assert_equal "normal string 42", Binary.sanitize_sql_like("normal string 42") end def test_sanitize_sql_like_with_custom_escape_character - assert_equal "100!%", Binary.send(:sanitize_sql_like, "100%", "!") - assert_equal "snake!_cased!_string", Binary.send(:sanitize_sql_like, "snake_cased_string", "!") - assert_equal "great!!", Binary.send(:sanitize_sql_like, "great!", "!") - assert_equal 'C:\\Programs\\MsPaint', Binary.send(:sanitize_sql_like, 'C:\\Programs\\MsPaint', "!") - assert_equal "normal string 42", Binary.send(:sanitize_sql_like, "normal string 42", "!") + assert_equal "100!%", Binary.sanitize_sql_like("100%", "!") + assert_equal "snake!_cased!_string", Binary.sanitize_sql_like("snake_cased_string", "!") + assert_equal "great!!", Binary.sanitize_sql_like("great!", "!") + assert_equal 'C:\\Programs\\MsPaint', Binary.sanitize_sql_like('C:\\Programs\\MsPaint', "!") + assert_equal "normal string 42", Binary.sanitize_sql_like("normal string 42", "!") end def test_sanitize_sql_like_example_use_case diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index ac5092f1c1..a612ce9bb2 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -177,14 +177,14 @@ class SchemaDumperTest < ActiveRecord::TestCase def test_schema_dumps_index_columns_in_right_order index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*company_index/).first.strip - if current_adapter?(:PostgreSQLAdapter, :SQLite3Adapter) - assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", order: { rating: :desc }', index_definition - elsif current_adapter?(:Mysql2Adapter) + if current_adapter?(:Mysql2Adapter) if ActiveRecord::Base.connection.supports_index_sort_order? assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", length: { type: 10 }, order: { rating: :desc }', index_definition else assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", length: { type: 10 }', index_definition end + elsif ActiveRecord::Base.connection.supports_index_sort_order? + assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", order: { rating: :desc }', index_definition else assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index"', index_definition end @@ -199,6 +199,24 @@ class SchemaDumperTest < ActiveRecord::TestCase end end + def test_schema_dumps_index_sort_order + index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*_name_and_rating/).first.strip + if ActiveRecord::Base.connection.supports_index_sort_order? + assert_equal 't.index ["name", "rating"], name: "index_companies_on_name_and_rating", order: :desc', index_definition + else + assert_equal 't.index ["name", "rating"], name: "index_companies_on_name_and_rating"', index_definition + end + end + + def test_schema_dumps_index_length + index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*_name_and_description/).first.strip + if current_adapter?(:Mysql2Adapter) + assert_equal 't.index ["name", "description"], name: "index_companies_on_name_and_description", length: 10', index_definition + else + assert_equal 't.index ["name", "description"], name: "index_companies_on_name_and_description"', index_definition + end + end + def test_schema_dump_should_honor_nonstandard_primary_keys output = standard_dump match = output.match(%r{create_table "movies"(.*)do}) diff --git a/activerecord/test/cases/statement_cache_test.rb b/activerecord/test/cases/statement_cache_test.rb index 1f715e41a6..ad6cd198e2 100644 --- a/activerecord/test/cases/statement_cache_test.rb +++ b/activerecord/test/cases/statement_cache_test.rb @@ -12,7 +12,6 @@ module ActiveRecord @connection = ActiveRecord::Base.connection end - #Cache v 1.1 tests def test_statement_cache Book.create(name: "my book") Book.create(name: "my other book") @@ -51,8 +50,6 @@ module ActiveRecord assert_equal("my other book", b.name) end - #End - def test_statement_cache_with_simple_statement cache = ActiveRecord::StatementCache.create(Book.connection) do |params| Book.where(name: "my book").where("author_id > 3") diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index 5a094ead42..fd381f229f 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -30,13 +30,30 @@ module ActiveRecord def test_raises_an_error_when_called_with_protected_environment ActiveRecord::Migrator.stubs(:current_version).returns(1) - protected_environments = ActiveRecord::Base.protected_environments.dup + protected_environments = ActiveRecord::Base.protected_environments current_env = ActiveRecord::Migrator.current_environment assert_not_includes protected_environments, current_env # Assert no error ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! - ActiveRecord::Base.protected_environments << current_env + ActiveRecord::Base.protected_environments = [current_env] + assert_raise(ActiveRecord::ProtectedEnvironmentError) do + ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + end + ensure + ActiveRecord::Base.protected_environments = protected_environments + end + + def test_raises_an_error_when_called_with_protected_environment_which_name_is_a_symbol + ActiveRecord::Migrator.stubs(:current_version).returns(1) + + protected_environments = ActiveRecord::Base.protected_environments + current_env = ActiveRecord::Migrator.current_environment + assert_not_includes protected_environments, current_env + # Assert no error + ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + + ActiveRecord::Base.protected_environments = [current_env.to_sym] assert_raise(ActiveRecord::ProtectedEnvironmentError) do ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! end diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 5d3e2a175c..54e3f47e16 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -90,7 +90,7 @@ class TimestampTest < ActiveRecord::TestCase @developer.touch(:created_at) end - assert !@developer.created_at_changed? , "created_at should not be changed" + assert !@developer.created_at_changed?, "created_at should not be changed" assert !@developer.changed?, "record should not be changed" assert_not_equal previously_created_at, @developer.created_at assert_not_equal @previously_updated_at, @developer.updated_at diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 7f84939027..14623c43d2 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -175,12 +175,12 @@ class ValidationsTest < ActiveRecord::TestCase ActiveModel::Name.new(self, nil, "Topic") end attribute :wibble, :decimal, scale: 2, precision: 9 - validates_numericality_of :wibble, greater_than_or_equal_to: BigDecimal.new("97.18") + validates_numericality_of :wibble, greater_than_or_equal_to: BigDecimal("97.18") end assert_not klass.new(wibble: "97.179").valid? assert_not klass.new(wibble: 97.179).valid? - assert_not klass.new(wibble: BigDecimal.new("97.179")).valid? + assert_not klass.new(wibble: BigDecimal("97.179")).valid? end def test_acceptance_validator_doesnt_require_db_connection diff --git a/activerecord/test/models/wheel.rb b/activerecord/test/models/wheel.rb index e05fb64477..8db57d181e 100644 --- a/activerecord/test/models/wheel.rb +++ b/activerecord/test/models/wheel.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true class Wheel < ActiveRecord::Base - belongs_to :wheelable, polymorphic: true, counter_cache: true + belongs_to :wheelable, polymorphic: true, counter_cache: true, touch: true end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index a4505a4892..3205c4c20a 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -123,7 +123,7 @@ ActiveRecord::Schema.define do create_table :cars, force: true do |t| t.string :name t.integer :engines_count - t.integer :wheels_count + t.integer :wheels_count, default: 0 t.column :lock_version, :integer, null: false, default: 0 t.timestamps null: false end @@ -205,6 +205,8 @@ ActiveRecord::Schema.define do t.bigint :rating, default: 1 t.integer :account_id t.string :description, default: "" + t.index [:name, :rating], order: :desc + t.index [:name, :description], length: 10 t.index [:firm_id, :type, :rating], name: "company_index", length: { type: 10 }, order: { rating: :desc } t.index [:firm_id, :type], name: "company_partial_index", where: "(rating > 10)" t.index :name, name: "company_name_index", using: :btree @@ -962,6 +964,7 @@ ActiveRecord::Schema.define do end create_table :wheels, force: true do |t| + t.integer :size t.references :wheelable, polymorphic: true end diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 2aa05d665e..acaf22fac1 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -270,7 +270,8 @@ class ActiveStorage::Blob < ActiveRecord::Base # deleted as well or you will essentially have a dead reference. It's recommended to use the +#purge+ and +#purge_later+ # methods in most circumstances. def delete - service.delete key + service.delete(key) + service.delete_prefixed("variants/#{key}/") if image? end # Deletes the file on the service and then destroys the blob record. This is the recommended way to dispose of unwanted diff --git a/activestorage/app/models/active_storage/filename.rb b/activestorage/app/models/active_storage/filename.rb index 79d55dc889..b9413dec95 100644 --- a/activestorage/app/models/active_storage/filename.rb +++ b/activestorage/app/models/active_storage/filename.rb @@ -50,7 +50,7 @@ class ActiveStorage::Filename @filename.encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;/\t\r\n\\", "-") end - def parameters + def parameters #:nodoc: Parameters.new self end diff --git a/activestorage/app/models/active_storage/filename/parameters.rb b/activestorage/app/models/active_storage/filename/parameters.rb index 58ce198d38..fb9ea10e49 100644 --- a/activestorage/app/models/active_storage/filename/parameters.rb +++ b/activestorage/app/models/active_storage/filename/parameters.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class ActiveStorage::Filename::Parameters +class ActiveStorage::Filename::Parameters #:nodoc: attr_reader :filename def initialize(filename) diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index 13bad87cac..fc4305dcc5 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -46,11 +46,13 @@ class ActiveStorage::Variation # Accepts an open MiniMagick image instance, like what's returned by <tt>MiniMagick::Image.read(io)</tt>, # and performs the +transformations+ against it. The transformed image instance is then returned. def transform(image) - transformations.each do |(method, argument)| - if eligible_argument?(argument) - image.public_send(method, argument) - else - image.public_send(method) + transformations.each do |method, argument| + image.mogrify do |command| + if eligible_argument?(argument) + command.public_send(method, argument) + else + command.public_send(method) + end end end end diff --git a/activestorage/config/routes.rb b/activestorage/config/routes.rb index 1eae21445a..ad9640ce03 100644 --- a/activestorage/config/routes.rb +++ b/activestorage/config/routes.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true Rails.application.routes.draw do - get "/rails/active_storage/blobs/:signed_id/*filename" => "active_storage/blobs#show", as: :rails_service_blob, internal: true + get "/rails/active_storage/blobs/:signed_id/*filename" => "active_storage/blobs#show", as: :rails_service_blob direct :rails_blob do |blob, options| route_for(:rails_service_blob, blob.signed_id, blob.filename, options) @@ -11,7 +11,7 @@ Rails.application.routes.draw do resolve("ActiveStorage::Attachment") { |attachment, options| route_for(:rails_blob, attachment.blob, options) } - get "/rails/active_storage/variants/:signed_blob_id/:variation_key/*filename" => "active_storage/variants#show", as: :rails_blob_variation, internal: true + get "/rails/active_storage/variants/:signed_blob_id/:variation_key/*filename" => "active_storage/variants#show", as: :rails_blob_variation direct :rails_variant do |variant, options| signed_blob_id = variant.blob.signed_id @@ -24,7 +24,7 @@ Rails.application.routes.draw do resolve("ActiveStorage::Variant") { |variant, options| route_for(:rails_variant, variant, options) } - get "/rails/active_storage/previews/:signed_blob_id/:variation_key/*filename" => "active_storage/previews#show", as: :rails_blob_preview, internal: true + get "/rails/active_storage/previews/:signed_blob_id/:variation_key/*filename" => "active_storage/previews#show", as: :rails_blob_preview direct :rails_preview do |preview, options| signed_blob_id = preview.blob.signed_id @@ -37,7 +37,7 @@ Rails.application.routes.draw do resolve("ActiveStorage::Preview") { |preview, options| route_for(:rails_preview, preview, options) } - get "/rails/active_storage/disk/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_service, internal: true - put "/rails/active_storage/disk/:encoded_token" => "active_storage/disk#update", as: :update_rails_disk_service, internal: true - post "/rails/active_storage/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads, internal: true + get "/rails/active_storage/disk/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_service + put "/rails/active_storage/disk/:encoded_token" => "active_storage/disk#update", as: :update_rails_disk_service + post "/rails/active_storage/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads end diff --git a/activestorage/lib/active_storage/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb index 408b5e58e9..1c144baa37 100644 --- a/activestorage/lib/active_storage/analyzer/video_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb @@ -19,6 +19,8 @@ module ActiveStorage # This analyzer requires the {ffmpeg}[https://www.ffmpeg.org] system library, which is not provided by Rails. You must # install ffmpeg yourself to use this analyzer. class Analyzer::VideoAnalyzer < Analyzer + class_attribute :ffprobe_path, default: "ffprobe" + def self.accept?(blob) blob.video? end @@ -29,10 +31,18 @@ module ActiveStorage private def width - Integer(video_stream["width"]) if video_stream["width"] + rotated? ? raw_height : raw_width end def height + rotated? ? raw_width : raw_height + end + + def raw_width + Integer(video_stream["width"]) if video_stream["width"] + end + + def raw_height Integer(video_stream["height"]) if video_stream["height"] end @@ -50,6 +60,10 @@ module ActiveStorage end end + def rotated? + angle == 90 || angle == 270 + end + def tags @tags ||= video_stream["tags"] || {} @@ -68,7 +82,7 @@ module ActiveStorage end def probe_from(file) - IO.popen([ "ffprobe", "-print_format", "json", "-show_streams", "-v", "error", file.path ]) do |output| + IO.popen([ ffprobe_path, "-print_format", "json", "-show_streams", "-v", "error", file.path ]) do |output| JSON.parse(output.read) end rescue Errno::ENOENT diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 6cf6635c4f..b870e6d4d6 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -15,7 +15,8 @@ module ActiveStorage config.active_storage = ActiveSupport::OrderedOptions.new config.active_storage.previewers = [ ActiveStorage::Previewer::PDFPreviewer, ActiveStorage::Previewer::VideoPreviewer ] - config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer ] + config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer ] + config.active_storage.paths = ActiveSupport::OrderedOptions.new config.eager_load_namespaces << ActiveStorage @@ -68,5 +69,21 @@ module ActiveStorage end end end + + initializer "active_storage.paths" do + config.after_initialize do |app| + if ffprobe_path = app.config.active_storage.paths.ffprobe + ActiveStorage::Analyzer::VideoAnalyzer.ffprobe_path = ffprobe_path + end + + if ffmpeg_path = app.config.active_storage.paths.ffmpeg + ActiveStorage::Previewer::VideoPreviewer.ffmpeg_path = ffmpeg_path + end + + if mutool_path = app.config.active_storage.paths.mutool + ActiveStorage::Previewer::PDFPreviewer.mutool_path = mutool_path + end + end + end end end diff --git a/activestorage/lib/active_storage/log_subscriber.rb b/activestorage/lib/active_storage/log_subscriber.rb index 5cbf4bd1a5..a4e148c1a5 100644 --- a/activestorage/lib/active_storage/log_subscriber.rb +++ b/activestorage/lib/active_storage/log_subscriber.rb @@ -18,6 +18,10 @@ module ActiveStorage info event, color("Deleted file from key: #{key_in(event)}", RED) end + def service_delete_prefixed(event) + info event, color("Deleted files by key prefix: #{event.payload[:prefix]}", RED) + end + def service_exist(event) debug event, color("Checked if file exists at key: #{key_in(event)} (#{event.payload[:exist] ? "yes" : "no"})", BLUE) end diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb index ed75bae3b5..3d485988e9 100644 --- a/activestorage/lib/active_storage/previewer.rb +++ b/activestorage/lib/active_storage/previewer.rb @@ -54,5 +54,9 @@ module ActiveStorage IO.popen(argv) { |out| IO.copy_stream(out, to) } to.rewind end + + def logger + ActiveStorage.logger + end end end diff --git a/activestorage/lib/active_storage/previewer/pdf_previewer.rb b/activestorage/lib/active_storage/previewer/pdf_previewer.rb index a2f05c74a6..b84aefcc9c 100644 --- a/activestorage/lib/active_storage/previewer/pdf_previewer.rb +++ b/activestorage/lib/active_storage/previewer/pdf_previewer.rb @@ -2,16 +2,23 @@ module ActiveStorage class Previewer::PDFPreviewer < Previewer + class_attribute :mutool_path, default: "mutool" + def self.accept?(blob) blob.content_type == "application/pdf" end def preview download_blob_to_tempfile do |input| - draw "mutool", "draw", "-F", "png", "-o", "-", input.path, "1" do |output| + draw_first_page_from input do |output| yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png" end end end + + private + def draw_first_page_from(file, &block) + draw mutool_path, "draw", "-F", "png", "-o", "-", file.path, "1", &block + end end end diff --git a/activestorage/lib/active_storage/previewer/video_previewer.rb b/activestorage/lib/active_storage/previewer/video_previewer.rb index 49f128d142..5d06e33f44 100644 --- a/activestorage/lib/active_storage/previewer/video_previewer.rb +++ b/activestorage/lib/active_storage/previewer/video_previewer.rb @@ -2,6 +2,8 @@ module ActiveStorage class Previewer::VideoPreviewer < Previewer + class_attribute :ffmpeg_path, default: "ffmpeg" + def self.accept?(blob) blob.video? end @@ -16,7 +18,7 @@ module ActiveStorage private def draw_relevant_frame_from(file, &block) - draw "ffmpeg", "-i", file.path, "-y", "-vcodec", "png", + draw ffmpeg_path, "-i", file.path, "-y", "-vcodec", "png", "-vf", "thumbnail", "-vframes", "1", "-f", "image2", "-", &block end end diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb index aa150e4d8a..c8f675db86 100644 --- a/activestorage/lib/active_storage/service.rb +++ b/activestorage/lib/active_storage/service.rb @@ -78,6 +78,11 @@ module ActiveStorage raise NotImplementedError end + # Delete files at keys starting with the +prefix+. + def delete_prefixed(prefix) + raise NotImplementedError + end + # Return +true+ if a file exists at the +key+. def exist?(key) raise NotImplementedError @@ -104,10 +109,10 @@ module ActiveStorage end private - def instrument(operation, key, payload = {}, &block) + def instrument(operation, payload = {}, &block) ActiveSupport::Notifications.instrument( "service_#{operation}.active_storage", - payload.merge(key: key, service: service_name), &block) + payload.merge(service: service_name), &block) end def service_name diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index f3877ad9c9..0a9eb7f23d 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -19,7 +19,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do begin blobs.create_block_blob(container, key, io, content_md5: checksum) rescue Azure::Core::Http::HTTPError @@ -30,11 +30,11 @@ module ActiveStorage def download(key, &block) if block_given? - instrument :streaming_download, key do + instrument :streaming_download, key: key do stream(key, &block) end else - instrument :download, key do + instrument :download, key: key do _, io = blobs.get_blob(container, key) io.force_encoding(Encoding::BINARY) end @@ -42,17 +42,33 @@ module ActiveStorage end def delete(key) - instrument :delete, key do + instrument :delete, key: key do begin blobs.delete_blob(container, key) rescue Azure::Core::Http::HTTPError - false + # Ignore files already deleted + end + end + end + + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + marker = nil + + loop do + results = blobs.list_blobs(container, prefix: prefix, marker: marker) + + results.each do |blob| + blobs.delete_blob(container, blob.name) + end + + break unless marker = results.continuation_token.presence end end end def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = blob_for(key).present? payload[:exist] = answer answer @@ -60,7 +76,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, disposition:, content_type:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| base_url = url_for(key) generated_url = signer.signed_uri( URI(base_url), false, @@ -77,7 +93,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| base_url = url_for(key) generated_url = signer.signed_uri(URI(base_url), false, permissions: "rw", expiry: format_expiry(expires_in)).to_s diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 52eaba4e7b..a8728c5bc3 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -16,7 +16,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do IO.copy_stream(io, make_path_for(key)) ensure_integrity_of(key, checksum) if checksum end @@ -24,7 +24,7 @@ module ActiveStorage def download(key) if block_given? - instrument :streaming_download, key do + instrument :streaming_download, key: key do File.open(path_for(key), "rb") do |file| while data = file.read(64.kilobytes) yield data @@ -32,14 +32,14 @@ module ActiveStorage end end else - instrument :download, key do + instrument :download, key: key do File.binread path_for(key) end end end def delete(key) - instrument :delete, key do + instrument :delete, key: key do begin File.delete path_for(key) rescue Errno::ENOENT @@ -48,8 +48,16 @@ module ActiveStorage end end + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + Dir.glob(path_for("#{prefix}*")).each do |path| + FileUtils.rm_rf(path) + end + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = File.exist? path_for(key) payload[:exist] = answer answer @@ -57,7 +65,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, disposition:, content_type:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| verified_key_with_expiration = ActiveStorage.verifier.generate(key, expires_in: expires_in, purpose: :blob_key) generated_url = @@ -77,7 +85,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| verified_token_with_expiration = ActiveStorage.verifier.generate( { key: key, diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index fd9916634a..6f6f4105fe 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -14,9 +14,15 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do begin - bucket.create_file(io, key, md5: checksum) + # The official GCS client library doesn't allow us to create a file with no Content-Type metadata. + # We need the file we create to have no Content-Type so we can control it via the response-content-type + # param in signed URLs. Workaround: let the GCS client create the file with an inferred + # Content-Type (usually "application/octet-stream") then clear it. + bucket.create_file(io, key, md5: checksum).update do |file| + file.content_type = nil + end rescue Google::Cloud::InvalidArgumentError raise ActiveStorage::IntegrityError end @@ -25,7 +31,7 @@ module ActiveStorage # FIXME: Download in chunks when given a block. def download(key) - instrument :download, key do + instrument :download, key: key do io = file_for(key).download io.rewind @@ -38,7 +44,7 @@ module ActiveStorage end def delete(key) - instrument :delete, key do + instrument :delete, key: key do begin file_for(key).delete rescue Google::Cloud::NotFoundError @@ -47,8 +53,14 @@ module ActiveStorage end end + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + bucket.files(prefix: prefix).all(&:delete) + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = file_for(key).exists? payload[:exist] = answer answer @@ -56,7 +68,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, content_type:, disposition:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = file_for(key).signed_url expires: expires_in, query: { "response-content-disposition" => content_disposition_with(type: disposition, filename: filename), "response-content-type" => content_type @@ -69,7 +81,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = bucket.signed_url key, method: "PUT", expires: expires_in, content_type: content_type, content_md5: checksum diff --git a/activestorage/lib/active_storage/service/mirror_service.rb b/activestorage/lib/active_storage/service/mirror_service.rb index 39e922f7ab..7eca8ce7f4 100644 --- a/activestorage/lib/active_storage/service/mirror_service.rb +++ b/activestorage/lib/active_storage/service/mirror_service.rb @@ -35,6 +35,11 @@ module ActiveStorage perform_across_services :delete, key end + # Delete files at keys starting with the +prefix+ on all services. + def delete_prefixed(prefix) + perform_across_services :delete_prefixed, prefix + end + private def each_service(&block) [ primary, *mirrors ].each(&block) diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 6957119780..c95672f338 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -17,7 +17,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do begin object_for(key).put(upload_options.merge(body: io, content_md5: checksum)) rescue Aws::S3::Errors::BadDigest @@ -28,24 +28,30 @@ module ActiveStorage def download(key, &block) if block_given? - instrument :streaming_download, key do + instrument :streaming_download, key: key do stream(key, &block) end else - instrument :download, key do + instrument :download, key: key do object_for(key).get.body.read.force_encoding(Encoding::BINARY) end end end def delete(key) - instrument :delete, key do + instrument :delete, key: key do object_for(key).delete end end + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + bucket.objects(prefix: prefix).batch_delete! + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = object_for(key).exists? payload[:exist] = answer answer @@ -53,7 +59,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, disposition:, content_type:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = object_for(key).presigned_url :get, expires_in: expires_in.to_i, response_content_disposition: content_disposition_with(type: disposition, filename: filename), response_content_type: content_type @@ -65,7 +71,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = object_for(key).presigned_url :put, expires_in: expires_in.to_i, content_type: content_type, content_length: content_length, content_md5: checksum diff --git a/activestorage/lib/tasks/activestorage.rake b/activestorage/lib/tasks/activestorage.rake index ef923e5926..296e91afa1 100644 --- a/activestorage/lib/tasks/activestorage.rake +++ b/activestorage/lib/tasks/activestorage.rake @@ -3,6 +3,10 @@ namespace :active_storage do desc "Copy over the migration needed to the application" task install: :environment do - Rake::Task["active_storage:install:migrations"].invoke + if Rake::Task.task_defined?("active_storage:install:migrations") + Rake::Task["active_storage:install:migrations"].invoke + else + Rake::Task["app:active_storage:install:migrations"].invoke + end end end diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb index 4a3c4a8bfc..b3b9c97fe4 100644 --- a/activestorage/test/analyzer/video_analyzer_test.rb +++ b/activestorage/test/analyzer/video_analyzer_test.rb @@ -21,8 +21,8 @@ class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase blob = create_file_blob(filename: "rotated_video.mp4", content_type: "video/mp4") metadata = blob.tap(&:analyze).metadata - assert_equal 640, metadata[:width] - assert_equal 480, metadata[:height] + assert_equal 480, metadata[:width] + assert_equal 640, metadata[:height] assert_equal [4, 3], metadata[:aspect_ratio] assert_equal 5.227975, metadata[:duration] assert_equal 90, metadata[:angle] diff --git a/activestorage/test/dummy/config/application.rb b/activestorage/test/dummy/config/application.rb index 06cbc453a2..bd14ac0b1a 100644 --- a/activestorage/test/dummy/config/application.rb +++ b/activestorage/test/dummy/config/application.rb @@ -10,10 +10,6 @@ require "action_controller/railtie" require "action_view/railtie" require "sprockets/railtie" require "active_storage/engine" -#require "action_mailer/railtie" -#require "rails/test_unit/railtie" -#require "action_cable/engine" - Bundler.require(*Rails.groups) diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index 6e815997ba..f94e65ed77 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -41,13 +41,21 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end - test "purge removes from external service" do + test "purge deletes file from external service" do blob = create_blob blob.purge assert_not ActiveStorage::Blob.service.exist?(blob.key) end + test "purge deletes variants from external service" do + blob = create_file_blob + variant = blob.variant(resize: "100>").processed + + blob.purge + assert_not ActiveStorage::Blob.service.exist?(variant.key) + end + private def expected_url_for(blob, disposition: :inline) query_string = { content_type: blob.content_type, disposition: "#{disposition}; #{blob.filename.parameters}" }.to_param diff --git a/activestorage/test/service/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb index 1860149da9..7efcd60fb7 100644 --- a/activestorage/test/service/gcs_service_test.rb +++ b/activestorage/test/service/gcs_service_test.rb @@ -35,6 +35,20 @@ if SERVICE_CONFIGURATIONS[:gcs] assert_match(/storage\.googleapis\.com\/.*response-content-disposition=inline.*test\.txt.*response-content-type=text%2Fplain/, @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")) end + + test "signed URL response headers" do + begin + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data)) + + url = @service.url(key, expires_in: 2.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain") + response = Net::HTTP.get_response(URI(url)) + assert_equal "text/plain", response.header["Content-Type"] + ensure + @service.delete key + end + end end else puts "Skipping GCS Service tests because no GCS configuration was supplied" diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb index ade91ab89a..ce28c4393a 100644 --- a/activestorage/test/service/shared_service_tests.rb +++ b/activestorage/test/service/shared_service_tests.rb @@ -75,5 +75,22 @@ module ActiveStorage::Service::SharedServiceTests @service.delete SecureRandom.base58(24) end end + + test "deleting by prefix" do + begin + @service.upload("a/a/a", StringIO.new(FIXTURE_DATA)) + @service.upload("a/a/b", StringIO.new(FIXTURE_DATA)) + @service.upload("a/b/a", StringIO.new(FIXTURE_DATA)) + + @service.delete_prefixed("a/a/") + assert_not @service.exist?("a/a/a") + assert_not @service.exist?("a/a/b") + assert @service.exist?("a/b/a") + ensure + @service.delete("a/a/a") + @service.delete("a/a/b") + @service.delete("a/b/a") + end + end end end diff --git a/activestorage/webpack.config.js b/activestorage/webpack.config.js index 92c4530e7f..3a50eef470 100644 --- a/activestorage/webpack.config.js +++ b/activestorage/webpack.config.js @@ -1,4 +1,3 @@ -const webpack = require("webpack") const path = require("path") module.exports = { diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 7663fca347..abbadd404f 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -74,7 +74,7 @@ config.cache_store = :redis_cache_store # Supports all common cache store options (:namespace, :compress, - # :compress_threshold, :expires_in, :race_condition_tool) and all + # :compress_threshold, :expires_in, :race_condition_ttl) and all # Redis options. cache_password = Rails.application.secrets.redis_cache_password config.cache_store = :redis_cache_store, driver: :hiredis, diff --git a/activesupport/lib/active_support/core_ext/module/concerning.rb b/activesupport/lib/active_support/core_ext/module/concerning.rb index 370a948eea..800bf213cc 100644 --- a/activesupport/lib/active_support/core_ext/module/concerning.rb +++ b/activesupport/lib/active_support/core_ext/module/concerning.rb @@ -22,7 +22,7 @@ class Module # # == Using comments: # - # class Todo + # class Todo < ApplicationRecord # # Other todo implementation # # ... # @@ -42,7 +42,7 @@ class Module # # Noisy syntax. # - # class Todo + # class Todo < ApplicationRecord # # Other todo implementation # # ... # @@ -70,7 +70,7 @@ class Module # increased overhead can be a reasonable tradeoff even if it reduces our # at-a-glance perception of how things work. # - # class Todo + # class Todo < ApplicationRecord # # Other todo implementation # # ... # @@ -82,7 +82,7 @@ class Module # By quieting the mix-in noise, we arrive at a natural, low-ceremony way to # separate bite-sized concerns. # - # class Todo + # class Todo < ApplicationRecord # # Other todo implementation # # ... # @@ -101,7 +101,7 @@ class Module # end # # Todo.ancestors - # # => [Todo, Todo::EventTracking, Object] + # # => [Todo, Todo::EventTracking, ApplicationRecord, Object] # # This small step has some wonderful ripple effects. We can # * grok the behavior of our class in one glance, diff --git a/activesupport/lib/active_support/deprecation/method_wrappers.rb b/activesupport/lib/active_support/deprecation/method_wrappers.rb index c4b78102eb..5be893d281 100644 --- a/activesupport/lib/active_support/deprecation/method_wrappers.rb +++ b/activesupport/lib/active_support/deprecation/method_wrappers.rb @@ -60,6 +60,13 @@ module ActiveSupport deprecator.deprecation_warning(method_name, options[method_name]) super(*args, &block) end + + case + when target_module.protected_method_defined?(method_name) + protected method_name + when target_module.private_method_defined?(method_name) + private method_name + end end end diff --git a/activesupport/lib/active_support/testing/time_helpers.rb b/activesupport/lib/active_support/testing/time_helpers.rb index 8c620e7f8c..998a51a34c 100644 --- a/activesupport/lib/active_support/testing/time_helpers.rb +++ b/activesupport/lib/active_support/testing/time_helpers.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "active_support/core_ext/module/redefine_method" require "active_support/core_ext/string/strip" # for strip_heredoc require "active_support/core_ext/time/calculations" require "concurrent/map" @@ -43,7 +44,7 @@ module ActiveSupport def unstub_object(stub) singleton_class = stub.object.singleton_class - singleton_class.send :undef_method, stub.method_name + singleton_class.send :silence_redefinition_of_method, stub.method_name singleton_class.send :alias_method, stub.method_name, stub.original_method singleton_class.send :undef_method, stub.original_method end diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb index 988de9207f..7f684f7a0f 100644 --- a/activesupport/test/cache/stores/redis_cache_store_test.rb +++ b/activesupport/test/cache/stores/redis_cache_store_test.rb @@ -88,7 +88,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests @namespace = "namespace" @cache = ActiveSupport::Cache::RedisCacheStore.new(timeout: 0.1, namespace: @namespace, expires_in: 60) - #@cache.logger = Logger.new($stdout) # For test debugging + # @cache.logger = Logger.new($stdout) # For test debugging # For LocalCacheBehavior tests @peek = ActiveSupport::Cache::RedisCacheStore.new(timeout: 0.1, namespace: @namespace) diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 5f894db46f..30f1632460 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -77,7 +77,7 @@ module CallbacksTest skip_callback :save, :after, :after_save_method, unless: :yes skip_callback :save, :after, :after_save_method, if: :no skip_callback :save, :before, :before_save_method, unless: :no - skip_callback :save, :before, CallbackClass , if: :yes + skip_callback :save, :before, CallbackClass, if: :yes def yes; true; end def no; false; end end diff --git a/activesupport/test/core_ext/array/grouping_test.rb b/activesupport/test/core_ext/array/grouping_test.rb index da9d4963d8..c182b91826 100644 --- a/activesupport/test/core_ext/array/grouping_test.rb +++ b/activesupport/test/core_ext/array/grouping_test.rb @@ -116,7 +116,7 @@ class SplitTest < ActiveSupport::TestCase def test_split_with_block a = (1..10).to_a assert_equal [[1, 2], [4, 5], [7, 8], [10]], a.split { |i| i % 3 == 0 } - assert_equal [1, 2, 3, 4, 5, 6, 7, 8, 9 , 10], a + assert_equal [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], a end def test_split_with_edge_values diff --git a/activesupport/test/core_ext/date_and_time_behavior.rb b/activesupport/test/core_ext/date_and_time_behavior.rb index 91b92043d0..1176ed647a 100644 --- a/activesupport/test/core_ext/date_and_time_behavior.rb +++ b/activesupport/test/core_ext/date_and_time_behavior.rb @@ -297,24 +297,24 @@ module DateAndTimeBehavior def test_beginning_of_week assert_equal date_time_init(2005, 1, 31, 0, 0, 0), date_time_init(2005, 2, 4, 10, 10, 10).beginning_of_week - assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 11, 28, 0, 0, 0).beginning_of_week #monday - assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 11, 29, 0, 0, 0).beginning_of_week #tuesday - assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 11, 30, 0, 0, 0).beginning_of_week #wednesday - assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 12, 01, 0, 0, 0).beginning_of_week #thursday - assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 12, 02, 0, 0, 0).beginning_of_week #friday - assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 12, 03, 0, 0, 0).beginning_of_week #saturday - assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 12, 04, 0, 0, 0).beginning_of_week #sunday + assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 11, 28, 0, 0, 0).beginning_of_week # monday + assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 11, 29, 0, 0, 0).beginning_of_week # tuesday + assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 11, 30, 0, 0, 0).beginning_of_week # wednesday + assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 12, 01, 0, 0, 0).beginning_of_week # thursday + assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 12, 02, 0, 0, 0).beginning_of_week # friday + assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 12, 03, 0, 0, 0).beginning_of_week # saturday + assert_equal date_time_init(2005, 11, 28, 0, 0, 0), date_time_init(2005, 12, 04, 0, 0, 0).beginning_of_week # sunday end def test_end_of_week assert_equal date_time_init(2008, 1, 6, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 12, 31, 10, 10, 10).end_of_week - assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 8, 27, 0, 0, 0).end_of_week #monday - assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 8, 28, 0, 0, 0).end_of_week #tuesday - assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 8, 29, 0, 0, 0).end_of_week #wednesday - assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 8, 30, 0, 0, 0).end_of_week #thursday - assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 8, 31, 0, 0, 0).end_of_week #friday - assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 9, 01, 0, 0, 0).end_of_week #saturday - assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 9, 02, 0, 0, 0).end_of_week #sunday + assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 8, 27, 0, 0, 0).end_of_week # monday + assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 8, 28, 0, 0, 0).end_of_week # tuesday + assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 8, 29, 0, 0, 0).end_of_week # wednesday + assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 8, 30, 0, 0, 0).end_of_week # thursday + assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 8, 31, 0, 0, 0).end_of_week # friday + assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 9, 01, 0, 0, 0).end_of_week # saturday + assert_equal date_time_init(2007, 9, 2, 23, 59, 59, Rational(999999999, 1000)), date_time_init(2007, 9, 02, 0, 0, 0).end_of_week # sunday end def test_end_of_month diff --git a/activesupport/test/core_ext/date_ext_test.rb b/activesupport/test/core_ext/date_ext_test.rb index 0c6f3f595a..23d17956df 100644 --- a/activesupport/test/core_ext/date_ext_test.rb +++ b/activesupport/test/core_ext/date_ext_test.rb @@ -95,11 +95,11 @@ class DateExtCalculationsTest < ActiveSupport::TestCase end def test_beginning_of_week_in_calendar_reform - assert_equal Date.new(1582, 10, 1), Date.new(1582, 10, 15).beginning_of_week #friday + assert_equal Date.new(1582, 10, 1), Date.new(1582, 10, 15).beginning_of_week # friday end def test_end_of_week_in_calendar_reform - assert_equal Date.new(1582, 10, 17), Date.new(1582, 10, 4).end_of_week #thursday + assert_equal Date.new(1582, 10, 17), Date.new(1582, 10, 4).end_of_week # thursday end def test_end_of_year @@ -144,7 +144,7 @@ class DateExtCalculationsTest < ActiveSupport::TestCase assert_equal Date.new(2012, 9, 28), Date.new(2005, 2, 28).advance(years: 7, months: 7) assert_equal Date.new(2013, 10, 3), Date.new(2005, 2, 28).advance(years: 7, months: 19, days: 5) assert_equal Date.new(2013, 10, 17), Date.new(2005, 2, 28).advance(years: 7, months: 19, weeks: 2, days: 5) - assert_equal Date.new(2005, 2, 28), Date.new(2004, 2, 29).advance(years: 1) #leap day plus one year + assert_equal Date.new(2005, 2, 28), Date.new(2004, 2, 29).advance(years: 1) # leap day plus one year end def test_advance_does_first_years_and_then_days diff --git a/activesupport/test/core_ext/date_time_ext_test.rb b/activesupport/test/core_ext/date_time_ext_test.rb index ed962803fa..f4c9dfcb25 100644 --- a/activesupport/test/core_ext/date_time_ext_test.rb +++ b/activesupport/test/core_ext/date_time_ext_test.rb @@ -187,7 +187,7 @@ class DateTimeExtCalculationsTest < ActiveSupport::TestCase assert_equal DateTime.civil(2013, 10, 3, 15, 15, 10), DateTime.civil(2005, 2, 28, 15, 15, 10).advance(years: 7, months: 19, days: 5) assert_equal DateTime.civil(2013, 10, 17, 15, 15, 10), DateTime.civil(2005, 2, 28, 15, 15, 10).advance(years: 7, months: 19, weeks: 2, days: 5) assert_equal DateTime.civil(2001, 12, 27, 15, 15, 10), DateTime.civil(2005, 2, 28, 15, 15, 10).advance(years: -3, months: -2, days: -1) - assert_equal DateTime.civil(2005, 2, 28, 15, 15, 10), DateTime.civil(2004, 2, 29, 15, 15, 10).advance(years: 1) #leap day plus one year + assert_equal DateTime.civil(2005, 2, 28, 15, 15, 10), DateTime.civil(2004, 2, 29, 15, 15, 10).advance(years: 1) # leap day plus one year assert_equal DateTime.civil(2005, 2, 28, 20, 15, 10), DateTime.civil(2005, 2, 28, 15, 15, 10).advance(hours: 5) assert_equal DateTime.civil(2005, 2, 28, 15, 22, 10), DateTime.civil(2005, 2, 28, 15, 15, 10).advance(minutes: 7) assert_equal DateTime.civil(2005, 2, 28, 15, 15, 19), DateTime.civil(2005, 2, 28, 15, 15, 10).advance(seconds: 9) diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index b3e0cd8bd0..dbee543644 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -160,7 +160,7 @@ class DurationTest < ActiveSupport::TestCase end def test_time_plus_duration_returns_same_time_datatype - twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone["Moscow"] , Time.utc(2016, 4, 28, 00, 45)) + twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone["Moscow"], Time.utc(2016, 4, 28, 00, 45)) now = Time.now.utc %w( second minute hour day week month year ).each do |unit| assert_equal((now + 1.send(unit)).class, Time, "Time + 1.#{unit} must be Time") diff --git a/activesupport/test/core_ext/numeric_ext_test.rb b/activesupport/test/core_ext/numeric_ext_test.rb index d38124b214..4b9073da54 100644 --- a/activesupport/test/core_ext/numeric_ext_test.rb +++ b/activesupport/test/core_ext/numeric_ext_test.rb @@ -302,7 +302,7 @@ class NumericExtFormattingTest < ActiveSupport::TestCase assert_equal "40 KB", 41100.to_s(:human_size, precision: 2) assert_equal "1.0 KB", kilobytes(1.0123).to_s(:human_size, precision: 2, strip_insignificant_zeros: false) assert_equal "1.012 KB", kilobytes(1.0123).to_s(:human_size, precision: 3, significant: false) - assert_equal "1 KB", kilobytes(1.0123).to_s(:human_size, precision: 0, significant: true) #ignores significant it precision is 0 + assert_equal "1 KB", kilobytes(1.0123).to_s(:human_size, precision: 0, significant: true) # ignores significant it precision is 0 end def test_to_s__human_size_with_custom_delimiter_and_separator @@ -330,17 +330,17 @@ class NumericExtFormattingTest < ActiveSupport::TestCase assert_equal "489.0 Thousand", 489000.to_s(:human, precision: 4, strip_insignificant_zeros: false) assert_equal "1.2346 Million", 1234567.to_s(:human, precision: 4, significant: false) assert_equal "1,2 Million", 1234567.to_s(:human, precision: 1, significant: false, separator: ",") - assert_equal "1 Million", 1234567.to_s(:human, precision: 0, significant: true, separator: ",") #significant forced to false + assert_equal "1 Million", 1234567.to_s(:human, precision: 0, significant: true, separator: ",") # significant forced to false end def test_number_to_human_with_custom_units - #Only integers + # Only integers volume = { unit: "ml", thousand: "lt", million: "m3" } assert_equal "123 lt", 123456.to_s(:human, units: volume) assert_equal "12 ml", 12.to_s(:human, units: volume) assert_equal "1.23 m3", 1234567.to_s(:human, units: volume) - #Including fractionals + # Including fractionals distance = { mili: "mm", centi: "cm", deci: "dm", unit: "m", ten: "dam", hundred: "hm", thousand: "km" } assert_equal "1.23 mm", 0.00123.to_s(:human, units: distance) assert_equal "1.23 cm", 0.0123.to_s(:human, units: distance) @@ -353,14 +353,14 @@ class NumericExtFormattingTest < ActiveSupport::TestCase assert_equal "1.23 km", 1230.to_s(:human, units: distance) assert_equal "12.3 km", 12300.to_s(:human, units: distance) - #The quantifiers don't need to be a continuous sequence + # The quantifiers don't need to be a continuous sequence gangster = { hundred: "hundred bucks", million: "thousand quids" } assert_equal "1 hundred bucks", 100.to_s(:human, units: gangster) assert_equal "25 hundred bucks", 2500.to_s(:human, units: gangster) assert_equal "25 thousand quids", 25000000.to_s(:human, units: gangster) assert_equal "12300 thousand quids", 12345000000.to_s(:human, units: gangster) - #Spaces are stripped from the resulting string + # Spaces are stripped from the resulting string assert_equal "4", 4.to_s(:human, units: { unit: "", ten: "tens " }) assert_equal "4.5 tens", 45.to_s(:human, units: { unit: "", ten: " tens " }) end diff --git a/activesupport/test/core_ext/range_ext_test.rb b/activesupport/test/core_ext/range_ext_test.rb index 049fac8fd4..903c173e59 100644 --- a/activesupport/test/core_ext/range_ext_test.rb +++ b/activesupport/test/core_ext/range_ext_test.rb @@ -106,26 +106,26 @@ class RangeTest < ActiveSupport::TestCase end def test_each_on_time_with_zone - twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone["Eastern Time (US & Canada)"] , Time.utc(2006, 11, 28, 10, 30)) + 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).each {} end end def test_step_on_time_with_zone - twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone["Eastern Time (US & Canada)"] , Time.utc(2006, 11, 28, 10, 30)) + 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).step(1) {} end end 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)) + twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone["Eastern Time (US & Canada)"], Time.utc(2006, 11, 28, 10, 30)) 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)) + 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 diff --git a/activesupport/test/core_ext/time_ext_test.rb b/activesupport/test/core_ext/time_ext_test.rb index 8cb17df01b..01cf1938be 100644 --- a/activesupport/test/core_ext/time_ext_test.rb +++ b/activesupport/test/core_ext/time_ext_test.rb @@ -451,7 +451,7 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase assert_equal Time.local(2013, 10, 3, 15, 15, 10), Time.local(2005, 2, 28, 15, 15, 10).advance(years: 7, months: 19, days: 5) assert_equal Time.local(2013, 10, 17, 15, 15, 10), Time.local(2005, 2, 28, 15, 15, 10).advance(years: 7, months: 19, weeks: 2, days: 5) assert_equal Time.local(2001, 12, 27, 15, 15, 10), Time.local(2005, 2, 28, 15, 15, 10).advance(years: -3, months: -2, days: -1) - assert_equal Time.local(2005, 2, 28, 15, 15, 10), Time.local(2004, 2, 29, 15, 15, 10).advance(years: 1) #leap day plus one year + assert_equal Time.local(2005, 2, 28, 15, 15, 10), Time.local(2004, 2, 29, 15, 15, 10).advance(years: 1) # leap day plus one year assert_equal Time.local(2005, 2, 28, 20, 15, 10), Time.local(2005, 2, 28, 15, 15, 10).advance(hours: 5) assert_equal Time.local(2005, 2, 28, 15, 22, 10), Time.local(2005, 2, 28, 15, 15, 10).advance(minutes: 7) assert_equal Time.local(2005, 2, 28, 15, 15, 19), Time.local(2005, 2, 28, 15, 15, 10).advance(seconds: 9) @@ -473,7 +473,7 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase assert_equal Time.utc(2013, 10, 3, 15, 15, 10), Time.utc(2005, 2, 22, 15, 15, 10).advance(years: 7, months: 19, days: 11) assert_equal Time.utc(2013, 10, 17, 15, 15, 10), Time.utc(2005, 2, 28, 15, 15, 10).advance(years: 7, months: 19, weeks: 2, days: 5) assert_equal Time.utc(2001, 12, 27, 15, 15, 10), Time.utc(2005, 2, 28, 15, 15, 10).advance(years: -3, months: -2, days: -1) - assert_equal Time.utc(2005, 2, 28, 15, 15, 10), Time.utc(2004, 2, 29, 15, 15, 10).advance(years: 1) #leap day plus one year + assert_equal Time.utc(2005, 2, 28, 15, 15, 10), Time.utc(2004, 2, 29, 15, 15, 10).advance(years: 1) # leap day plus one year assert_equal Time.utc(2005, 2, 28, 20, 15, 10), Time.utc(2005, 2, 28, 15, 15, 10).advance(hours: 5) assert_equal Time.utc(2005, 2, 28, 15, 22, 10), Time.utc(2005, 2, 28, 15, 15, 10).advance(minutes: 7) assert_equal Time.utc(2005, 2, 28, 15, 15, 19), Time.utc(2005, 2, 28, 15, 15, 10).advance(seconds: 9) @@ -495,7 +495,7 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase assert_equal Time.new(2013, 10, 3, 15, 15, 10, "-08:00"), Time.new(2005, 2, 22, 15, 15, 10, "-08:00").advance(years: 7, months: 19, days: 11) assert_equal Time.new(2013, 10, 17, 15, 15, 10, "-08:00"), Time.new(2005, 2, 28, 15, 15, 10, "-08:00").advance(years: 7, months: 19, weeks: 2, days: 5) assert_equal Time.new(2001, 12, 27, 15, 15, 10, "-08:00"), Time.new(2005, 2, 28, 15, 15, 10, "-08:00").advance(years: -3, months: -2, days: -1) - assert_equal Time.new(2005, 2, 28, 15, 15, 10, "-08:00"), Time.new(2004, 2, 29, 15, 15, 10, "-08:00").advance(years: 1) #leap day plus one year + assert_equal Time.new(2005, 2, 28, 15, 15, 10, "-08:00"), Time.new(2004, 2, 29, 15, 15, 10, "-08:00").advance(years: 1) # leap day plus one year assert_equal Time.new(2005, 2, 28, 20, 15, 10, "-08:00"), Time.new(2005, 2, 28, 15, 15, 10, "-08:00").advance(hours: 5) assert_equal Time.new(2005, 2, 28, 15, 22, 10, "-08:00"), Time.new(2005, 2, 28, 15, 15, 10, "-08:00").advance(minutes: 7) assert_equal Time.new(2005, 2, 28, 15, 15, 19, "-08:00"), Time.new(2005, 2, 28, 15, 15, 10, "-08:00").advance(seconds: 9) diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb index ab96568956..b25747eadb 100644 --- a/activesupport/test/core_ext/time_with_zone_test.rb +++ b/activesupport/test/core_ext/time_with_zone_test.rb @@ -84,7 +84,7 @@ class TimeWithZoneTest < ActiveSupport::TestCase def test_formatted_offset assert_equal "-05:00", @twz.formatted_offset - assert_equal "-04:00", ActiveSupport::TimeWithZone.new(Time.utc(2000, 6), @time_zone).formatted_offset #dst + assert_equal "-04:00", ActiveSupport::TimeWithZone.new(Time.utc(2000, 6), @time_zone).formatted_offset # dst end def test_dst? @@ -94,7 +94,7 @@ class TimeWithZoneTest < ActiveSupport::TestCase def test_zone assert_equal "EST", @twz.zone - assert_equal "EDT", ActiveSupport::TimeWithZone.new(Time.utc(2000, 6), @time_zone).zone #dst + assert_equal "EDT", ActiveSupport::TimeWithZone.new(Time.utc(2000, 6), @time_zone).zone # dst end def test_nsec @@ -307,13 +307,13 @@ class TimeWithZoneTest < ActiveSupport::TestCase end def test_plus_with_integer - assert_equal Time.utc(1999, 12, 31, 19, 0 , 5), (@twz + 5).time + assert_equal Time.utc(1999, 12, 31, 19, 0, 5), (@twz + 5).time end def test_plus_with_integer_when_self_wraps_datetime datetime = DateTime.civil(2000, 1, 1, 0) twz = ActiveSupport::TimeWithZone.new(datetime, @time_zone) - assert_equal DateTime.civil(1999, 12, 31, 19, 0 , 5), (twz + 5).time + assert_equal DateTime.civil(1999, 12, 31, 19, 0, 5), (twz + 5).time end def test_plus_when_crossing_time_class_limit @@ -322,21 +322,21 @@ class TimeWithZoneTest < ActiveSupport::TestCase end def test_plus_with_duration - assert_equal Time.utc(2000, 1, 5, 19, 0 , 0), (@twz + 5.days).time + assert_equal Time.utc(2000, 1, 5, 19, 0, 0), (@twz + 5.days).time end def test_minus_with_integer - assert_equal Time.utc(1999, 12, 31, 18, 59 , 55), (@twz - 5).time + assert_equal Time.utc(1999, 12, 31, 18, 59, 55), (@twz - 5).time end def test_minus_with_integer_when_self_wraps_datetime datetime = DateTime.civil(2000, 1, 1, 0) twz = ActiveSupport::TimeWithZone.new(datetime, @time_zone) - assert_equal DateTime.civil(1999, 12, 31, 18, 59 , 55), (twz - 5).time + assert_equal DateTime.civil(1999, 12, 31, 18, 59, 55), (twz - 5).time end def test_minus_with_duration - assert_equal Time.utc(1999, 12, 26, 19, 0 , 0), (@twz - 5.days).time + assert_equal Time.utc(1999, 12, 26, 19, 0, 0), (@twz - 5.days).time end def test_minus_with_time @@ -507,7 +507,7 @@ class TimeWithZoneTest < ActiveSupport::TestCase def test_method_missing_with_time_return_value assert_instance_of ActiveSupport::TimeWithZone, @twz.months_since(1) - assert_equal Time.utc(2000, 1, 31, 19, 0 , 0), @twz.months_since(1).time + assert_equal Time.utc(2000, 1, 31, 19, 0, 0), @twz.months_since(1).time end def test_marshal_dump_and_load diff --git a/activesupport/test/deprecation/method_wrappers_test.rb b/activesupport/test/deprecation/method_wrappers_test.rb index 04e2325754..439e117c1d 100644 --- a/activesupport/test/deprecation/method_wrappers_test.rb +++ b/activesupport/test/deprecation/method_wrappers_test.rb @@ -8,6 +8,16 @@ class MethodWrappersTest < ActiveSupport::TestCase @klass = Class.new do def new_method; "abc" end alias_method :old_method, :new_method + + protected + + def new_protected_method; "abc" end + alias_method :old_protected_method, :new_protected_method + + private + + def new_private_method; "abc" end + alias_method :old_private_method, :new_private_method end end @@ -33,4 +43,16 @@ class MethodWrappersTest < ActiveSupport::TestCase assert_deprecated(warning, deprecator) { assert_equal "abc", @klass.new.old_method } end + + def test_deprecate_methods_protected_method + ActiveSupport::Deprecation.deprecate_methods(@klass, old_protected_method: :new_protected_method) + + assert(@klass.protected_method_defined?(:old_protected_method)) + end + + def test_deprecate_methods_private_method + ActiveSupport::Deprecation.deprecate_methods(@klass, old_private_method: :new_private_method) + + assert(@klass.private_method_defined?(:old_private_method)) + end end diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index d904f79ccf..340a2abf75 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -187,7 +187,7 @@ class TestJSONEncoding < ActiveSupport::TestCase def test_array_should_pass_encoding_options_to_children_in_as_json people = [ { name: "John", address: { city: "London", country: "UK" } }, - { name: "Jean", address: { city: "Paris" , country: "France" } } + { name: "Jean", address: { city: "Paris", country: "France" } } ] json = people.as_json only: [:address, :city] expected = [ @@ -201,7 +201,7 @@ class TestJSONEncoding < ActiveSupport::TestCase def test_array_should_pass_encoding_options_to_children_in_to_json people = [ { name: "John", address: { city: "London", country: "UK" } }, - { name: "Jean", address: { city: "Paris" , country: "France" } } + { name: "Jean", address: { city: "Paris", country: "France" } } ] json = people.to_json only: [:address, :city] @@ -213,7 +213,7 @@ class TestJSONEncoding < ActiveSupport::TestCase def initialize @people = [ { name: "John", address: { city: "London", country: "UK" } }, - { name: "Jean", address: { city: "Paris" , country: "France" } } + { name: "Jean", address: { city: "Paris", country: "France" } } ] end def each(*, &blk) diff --git a/activesupport/test/number_helper_i18n_test.rb b/activesupport/test/number_helper_i18n_test.rb index 6d6958be49..365fa96f4d 100644 --- a/activesupport/test/number_helper_i18n_test.rb +++ b/activesupport/test/number_helper_i18n_test.rb @@ -35,7 +35,7 @@ module ActiveSupport thousand: "t", million: "m", billion: "b", - trillion: "t" , + trillion: "t", quadrillion: "q" } } @@ -77,10 +77,10 @@ module ActiveSupport end def test_number_with_i18n_precision - #Delimiter was set to "" + # Delimiter was set to "" assert_equal("10000", number_to_rounded(10000, locale: "ts")) - #Precision inherited and significant was set + # Precision inherited and significant was set assert_equal("1.00", number_to_rounded(1.0, locale: "ts")) end @@ -90,7 +90,7 @@ module ActiveSupport end def test_number_with_i18n_delimiter - #Delimiter "," and separator "." + # Delimiter "," and separator "." assert_equal("1,000,000.234", number_to_delimited(1000000.234, locale: "ts")) end @@ -114,7 +114,7 @@ module ActiveSupport end def test_number_to_i18n_human_size - #b for bytes and k for kbytes + # b for bytes and k for kbytes assert_equal("2 k", number_to_human_size(2048, locale: "ts")) assert_equal("42 b", number_to_human_size(42, locale: "ts")) end @@ -125,11 +125,11 @@ module ActiveSupport end def test_number_to_human_with_default_translation_scope - #Using t for thousand + # Using t for thousand assert_equal "2 t", number_to_human(2000, locale: "ts") - #Significant was set to true with precision 2, using b for billion + # Significant was set to true with precision 2, using b for billion assert_equal "1.2 b", number_to_human(1234567890, locale: "ts") - #Using pluralization (Ten/Tens and Tenth/Tenths) + # Using pluralization (Ten/Tens and Tenth/Tenths) assert_equal "1 Tenth", number_to_human(0.1, locale: "ts") assert_equal "1.3 Tenth", number_to_human(0.134, locale: "ts") assert_equal "2 Tenths", number_to_human(0.2, locale: "ts") @@ -144,7 +144,7 @@ module ActiveSupport end def test_number_to_human_with_custom_translation_scope - #Significant was set to true with precision 2, with custom translated units + # Significant was set to true with precision 2, with custom translated units assert_equal "4.3 cm", number_to_human(0.0432, locale: "ts", units: :custom_units_for_number_to_human) end end diff --git a/activesupport/test/number_helper_test.rb b/activesupport/test/number_helper_test.rb index b4795b6ee1..16ccc5572c 100644 --- a/activesupport/test/number_helper_test.rb +++ b/activesupport/test/number_helper_test.rb @@ -260,7 +260,7 @@ module ActiveSupport assert_equal "40 KB", number_helper.number_to_human_size(41100, precision: 2) assert_equal "1.0 KB", number_helper.number_to_human_size(kilobytes(1.0123), precision: 2, strip_insignificant_zeros: false) assert_equal "1.012 KB", number_helper.number_to_human_size(kilobytes(1.0123), precision: 3, significant: false) - assert_equal "1 KB", number_helper.number_to_human_size(kilobytes(1.0123), precision: 0, significant: true) #ignores significant it precision is 0 + assert_equal "1 KB", number_helper.number_to_human_size(kilobytes(1.0123), precision: 0, significant: true) # ignores significant it precision is 0 end end @@ -292,7 +292,7 @@ module ActiveSupport assert_equal "489.0 Thousand", number_helper.number_to_human(489000, precision: 4, strip_insignificant_zeros: false) assert_equal "1.2346 Million", number_helper.number_to_human(1234567, precision: 4, significant: false) assert_equal "1,2 Million", number_helper.number_to_human(1234567, precision: 1, significant: false, separator: ",") - assert_equal "1 Million", number_helper.number_to_human(1234567, precision: 0, significant: true, separator: ",") #significant forced to false + assert_equal "1 Million", number_helper.number_to_human(1234567, precision: 0, significant: true, separator: ",") # significant forced to false assert_equal "1 Million", number_helper.number_to_human(999999) assert_equal "1 Billion", number_helper.number_to_human(999999999) end @@ -300,13 +300,13 @@ module ActiveSupport def test_number_to_human_with_custom_units [@instance_with_helpers, TestClassWithClassNumberHelpers, ActiveSupport::NumberHelper].each do |number_helper| - #Only integers + # Only integers volume = { unit: "ml", thousand: "lt", million: "m3" } assert_equal "123 lt", number_helper.number_to_human(123456, units: volume) assert_equal "12 ml", number_helper.number_to_human(12, units: volume) assert_equal "1.23 m3", number_helper.number_to_human(1234567, units: volume) - #Including fractionals + # Including fractionals distance = { mili: "mm", centi: "cm", deci: "dm", unit: "m", ten: "dam", hundred: "hm", thousand: "km" } assert_equal "1.23 mm", number_helper.number_to_human(0.00123, units: distance) assert_equal "1.23 cm", number_helper.number_to_human(0.0123, units: distance) @@ -319,7 +319,7 @@ module ActiveSupport assert_equal "1.23 km", number_helper.number_to_human(1230, units: distance) assert_equal "12.3 km", number_helper.number_to_human(12300, units: distance) - #The quantifiers don't need to be a continuous sequence + # The quantifiers don't need to be a continuous sequence gangster = { hundred: "hundred bucks", million: "thousand quids" } assert_equal "1 hundred bucks", number_helper.number_to_human(100, units: gangster) assert_equal "25 hundred bucks", number_helper.number_to_human(2500, units: gangster) @@ -329,11 +329,11 @@ module ActiveSupport assert_equal "25 thousand quids", number_helper.number_to_human(25000000, units: gangster) assert_equal "12300 thousand quids", number_helper.number_to_human(12345000000, units: gangster) - #Spaces are stripped from the resulting string + # Spaces are stripped from the resulting string assert_equal "4", number_helper.number_to_human(4, units: { unit: "", ten: "tens " }) assert_equal "4.5 tens", number_helper.number_to_human(45, units: { unit: "", ten: " tens " }) - #Uses only the provided units and does not try to use larger ones + # Uses only the provided units and does not try to use larger ones assert_equal "1000 kilometers", number_helper.number_to_human(1_000_000, units: { unit: "meter", thousand: "kilometers" }) end end diff --git a/activesupport/test/time_travel_test.rb b/activesupport/test/time_travel_test.rb index 4172c18ead..9c2c635f43 100644 --- a/activesupport/test/time_travel_test.rb +++ b/activesupport/test/time_travel_test.rb @@ -98,7 +98,7 @@ class TimeTravelTest < ActiveSupport::TestCase travel_to outer_expected_time do e = assert_raises(RuntimeError) do travel_to(inner_expected_time) do - #noop + # noop end end assert_match(/Calling `travel_to` with a block, when we have previously already made a call to `travel_to`, can lead to confusing time stubbing\./, e.message) diff --git a/activesupport/test/xml_mini/libxml_engine_test.rb b/activesupport/test/xml_mini/libxml_engine_test.rb index 5c13f493f1..3eef3946a3 100644 --- a/activesupport/test/xml_mini/libxml_engine_test.rb +++ b/activesupport/test/xml_mini/libxml_engine_test.rb @@ -6,7 +6,7 @@ XMLMiniEngineTest.run_with_gem("libxml") do class LibxmlEngineTest < XMLMiniEngineTest def setup super - LibXML::XML::Error.set_handler(&lambda { |error| }) #silence libxml, exceptions will do + LibXML::XML::Error.set_handler(&lambda { |error| }) # silence libxml, exceptions will do end private diff --git a/guides/source/3_2_release_notes.md b/guides/source/3_2_release_notes.md index f6571544f9..ae6eb27f35 100644 --- a/guides/source/3_2_release_notes.md +++ b/guides/source/3_2_release_notes.md @@ -36,7 +36,7 @@ TIP: Note that Ruby 1.8.7 p248 and p249 have marshalling bugs that crash Rails. * `coffee-rails ~> 3.2.1` * `uglifier >= 1.0.3` -* Rails 3.2 deprecates `vendor/plugins` and Rails 4.0 will remove them completely. You can start replacing these plugins by extracting them as gems and adding them in your Gemfile. If you choose not to make them gems, you can move them into, say, `lib/my_plugin/*` and add an appropriate initializer in `config/initializers/my_plugin.rb`. +* Rails 3.2 deprecates `vendor/plugins` and Rails 4.0 will remove them completely. You can start replacing these plugins by extracting them as gems and adding them in your `Gemfile`. If you choose not to make them gems, you can move them into, say, `lib/my_plugin/*` and add an appropriate initializer in `config/initializers/my_plugin.rb`. * There are a couple of new configuration changes you'd want to add in `config/environments/development.rb`: @@ -156,7 +156,7 @@ Railties will create indexes for `title` and `author` with the latter being a unique index. Some types such as decimal accept custom options. In the example, `price` will be a decimal column with precision and scale set to 7 and 2 respectively. -* Turn gem has been removed from default Gemfile. +* Turn gem has been removed from default `Gemfile`. * Remove old plugin generator `rails generate plugin` in favor of `rails plugin new` command. diff --git a/guides/source/4_1_release_notes.md b/guides/source/4_1_release_notes.md index 6bf65757ec..2c5e665e33 100644 --- a/guides/source/4_1_release_notes.md +++ b/guides/source/4_1_release_notes.md @@ -274,7 +274,7 @@ for detailed changes. * The [Spring application preloader](https://github.com/rails/spring) is now installed by default for new applications. It uses the development group of - the Gemfile, so will not be installed in + the `Gemfile`, so will not be installed in production. ([Pull Request](https://github.com/rails/rails/pull/12958)) * `BACKTRACE` environment variable to show unfiltered backtraces for test diff --git a/guides/source/4_2_release_notes.md b/guides/source/4_2_release_notes.md index 036a310ac8..7105df5634 100644 --- a/guides/source/4_2_release_notes.md +++ b/guides/source/4_2_release_notes.md @@ -368,7 +368,7 @@ Please refer to the [Changelog][railties] for detailed changes. ### Notable changes -* Introduced `web-console` in the default application Gemfile. +* Introduced `web-console` in the default application `Gemfile`. ([Pull Request](https://github.com/rails/rails/pull/11667)) * Added a `required` option to the model generator for associations. diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md index c1e02745de..fde2040173 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -149,10 +149,10 @@ end #### Jbuilder [Jbuilder](https://github.com/rails/jbuilder) is a gem that's -maintained by the Rails team and included in the default Rails Gemfile. +maintained by the Rails team and included in the default Rails `Gemfile`. It's similar to Builder, but is used to generate JSON, instead of XML. -If you don't have it, you can add the following to your Gemfile: +If you don't have it, you can add the following to your `Gemfile`: ```ruby gem 'jbuilder' diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 3786343fc3..4e28e31a53 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -801,7 +801,7 @@ The SQL that would be executed: SELECT * FROM articles WHERE id > 10 ORDER BY id DESC # Original query without `only` -SELECT "articles".* FROM "articles" WHERE (id > 10) ORDER BY id desc LIMIT 20 +SELECT * FROM articles WHERE id > 10 ORDER BY id DESC LIMIT 20 ``` @@ -820,14 +820,14 @@ Article.find(10).comments.reorder('name') The SQL that would be executed: ```sql -SELECT * FROM articles WHERE id = 10 +SELECT * FROM articles WHERE id = 10 LIMIT 1 SELECT * FROM comments WHERE article_id = 10 ORDER BY name ``` In the case where the `reorder` clause is not used, the SQL executed would be: ```sql -SELECT * FROM articles WHERE id = 10 +SELECT * FROM articles WHERE id = 10 LIMIT 1 SELECT * FROM comments WHERE article_id = 10 ORDER BY posted_at DESC ``` @@ -1091,7 +1091,7 @@ This produces: ```sql SELECT articles.* FROM articles - INNER JOIN categories ON articles.category_id = categories.id + INNER JOIN categories ON categories.id = articles.category_id INNER JOIN comments ON comments.article_id = articles.id ``` @@ -1871,14 +1871,14 @@ All calculation methods work directly on a model: ```ruby Client.count -# SELECT count(*) AS count_all FROM clients +# SELECT COUNT(*) FROM clients ``` Or on a relation: ```ruby Client.where(first_name: 'Ryan').count -# SELECT count(*) AS count_all FROM clients WHERE (first_name = 'Ryan') +# SELECT COUNT(*) FROM clients WHERE (first_name = 'Ryan') ``` You can also use various finder methods on a relation for performing complex calculations: @@ -1890,9 +1890,9 @@ Client.includes("orders").where(first_name: 'Ryan', orders: { status: 'received' Which will execute: ```sql -SELECT count(DISTINCT clients.id) AS count_all FROM clients - LEFT OUTER JOIN orders ON orders.client_id = clients.id WHERE - (clients.first_name = 'Ryan' AND orders.status = 'received') +SELECT COUNT(DISTINCT clients.id) FROM clients + LEFT OUTER JOIN orders ON orders.client_id = clients.id + WHERE (clients.first_name = 'Ryan' AND orders.status = 'received') ``` ### Count diff --git a/guides/source/active_support_instrumentation.md b/guides/source/active_support_instrumentation.md index 25f78fd940..11c4a8222a 100644 --- a/guides/source/active_support_instrumentation.md +++ b/guides/source/active_support_instrumentation.md @@ -543,6 +543,13 @@ Active Storage | `:key` | Secure token | | `:service` | Name of the service | +### service_delete_prefixed.active_storage + +| Key | Value | +| ------------ | ------------------- | +| `:prefix` | Key prefix | +| `:service` | Name of the service | + ### service_exist.active_storage | Key | Value | diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index 805b0f0d62..e6d5aed135 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -35,7 +35,7 @@ rails new appname --skip-sprockets ``` Rails automatically adds the `sass-rails`, `coffee-rails` and `uglifier` -gems to your Gemfile, which are used by Sprockets for asset compression: +gems to your `Gemfile`, which are used by Sprockets for asset compression: ```ruby gem 'sass-rails' @@ -44,8 +44,8 @@ gem 'coffee-rails' ``` Using the `--skip-sprockets` option will prevent Rails from adding -them to your Gemfile, so if you later want to enable -the asset pipeline you will have to add those gems to your Gemfile. Also, +them to your `Gemfile`, so if you later want to enable +the asset pipeline you will have to add those gems to your `Gemfile`. Also, creating an application with the `--skip-sprockets` option will generate a slightly different `config/application.rb` file, with a require statement for the sprockets railtie that is commented-out. You will have to remove @@ -850,7 +850,7 @@ This mode uses more memory, performs more poorly than the default and is not recommended. If you are deploying a production application to a system without any -pre-existing JavaScript runtimes, you may want to add one to your Gemfile: +pre-existing JavaScript runtimes, you may want to add one to your `Gemfile`: ```ruby group :production do diff --git a/guides/source/caching_with_rails.md b/guides/source/caching_with_rails.md index 31bc478015..780e69c146 100644 --- a/guides/source/caching_with_rails.md +++ b/guides/source/caching_with_rails.md @@ -32,7 +32,7 @@ Basic Caching This is an introduction to three types of caching techniques: page, action and fragment caching. By default Rails provides fragment caching. In order to use page and action caching you will need to add `actionpack-page_caching` and -`actionpack-action_caching` to your Gemfile. +`actionpack-action_caching` to your `Gemfile`. By default, caching is only enabled in your production environment. To play around with caching locally you'll want to enable caching in your local diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 4bfcc1e21a..b1e472bb74 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -322,6 +322,10 @@ All these configuration options are delegated to the `I18n` library. * `config.active_record.schema_migrations_table_name` lets you set a string to be used as the name of the schema migrations table. +* `config.active_record.internal_metadata_table_name` lets you set a string to be used as the name of the internal metadata table. + +* `config.active_record.protected_environments` lets you set an array of names of environments where destructive actions should be prohibited. + * `config.active_record.pluralize_table_names` specifies whether Rails will look for singular or plural table names in the database. If set to `true` (the default), then the Customer class will use the `customers` table. If set to false, then the Customer class will use the `customer` table. * `config.active_record.default_timezone` determines whether to use `Time.local` (if set to `:local`) or `Time.utc` (if set to `:utc`) when pulling dates and times from the database. The default is `:utc`. @@ -399,7 +403,7 @@ by adding the following to your `application.rb` file: The schema dumper adds one additional configuration option: -* `ActiveRecord::SchemaDumper.ignore_tables` accepts an array of tables that should _not_ be included in any generated schema file. This setting is ignored unless `config.active_record.schema_format == :ruby`. +* `ActiveRecord::SchemaDumper.ignore_tables` accepts an array of tables that should _not_ be included in any generated schema file. ### Configuring Action Controller @@ -1005,11 +1009,11 @@ Deploying your application using a reverse proxy has definite advantages over tr Many modern web servers can be used as a proxy server to balance third-party elements such as caching servers or application servers. -One such application server you can use is [Unicorn](http://unicorn.bogomips.org/) to run behind a reverse proxy. +One such application server you can use is [Unicorn](https://bogomips.org/unicorn/) to run behind a reverse proxy. In this case, you would need to configure the proxy server (NGINX, Apache, etc) to accept connections from your application server (Unicorn). By default Unicorn will listen for TCP connections on port 8080, but you can change the port or configure it to use sockets instead. -You can find more information in the [Unicorn readme](http://unicorn.bogomips.org/README.html) and understand the [philosophy](http://unicorn.bogomips.org/PHILOSOPHY.html) behind it. +You can find more information in the [Unicorn readme](https://bogomips.org/unicorn/README.html) and understand the [philosophy](https://bogomips.org/unicorn/PHILOSOPHY.html) behind it. Once you've configured the application server, you must proxy requests to it by configuring your web server appropriately. For example your NGINX config may include: @@ -1037,7 +1041,7 @@ server { } ``` -Be sure to read the [NGINX documentation](http://nginx.org/en/docs/) for the most up-to-date information. +Be sure to read the [NGINX documentation](https://nginx.org/en/docs/) for the most up-to-date information. Rails Environment Settings diff --git a/guides/source/documents.yaml b/guides/source/documents.yaml index 2a4abab116..288519819d 100644 --- a/guides/source/documents.yaml +++ b/guides/source/documents.yaml @@ -72,7 +72,7 @@ url: active_support_core_extensions.html description: This guide documents the Ruby core extensions defined in Active Support. - - name: Rails Internationalization API + name: Rails Internationalization (I18n) API url: i18n.html description: This guide covers how to add internationalization to your applications. Your application will be able to translate content to different languages, change pluralization rules, use correct date formats for each country, and so on. - @@ -104,7 +104,7 @@ url: command_line.html description: This guide covers the command line tools provided by Rails. - - name: Asset Pipeline + name: The Asset Pipeline url: asset_pipeline.html description: This guide documents the asset pipeline. - @@ -151,7 +151,7 @@ url: rails_on_rack.html description: This guide covers Rails integration with Rack and interfacing with other Rack components. - - name: Creating and Customizing Rails Generators + name: Creating and Customizing Rails Generators & Templates url: generators.html description: This guide covers the process of adding a brand new generator to your extension or providing an alternative to an element of a built-in Rails generator (such as providing alternative test stubs for the scaffold generator). - @@ -183,7 +183,7 @@ name: Maintenance Policy documents: - - name: Maintenance Policy + name: Maintenance Policy for Ruby on Rails url: maintenance_policy.html description: What versions of Ruby on Rails are currently supported, and when to expect new versions. - diff --git a/guides/source/form_helpers.md b/guides/source/form_helpers.md index 4ce67df93a..53c567727f 100644 --- a/guides/source/form_helpers.md +++ b/guides/source/form_helpers.md @@ -1,7 +1,7 @@ **DO NOT READ THIS FILE ON GITHUB, GUIDES ARE PUBLISHED ON http://guides.rubyonrails.org.** -Form Helpers -============ +Action View Form Helpers +======================== Forms in web applications are an essential interface for user input. However, form markup can quickly become tedious to write and maintain because of the need to handle form control naming and its numerous attributes. Rails does away with this complexity by providing view helpers for generating form markup. However, since these helpers have different use cases, developers need to know the differences between the helper methods before putting them to use. @@ -920,7 +920,7 @@ When an association accepts nested attributes `fields_for` renders its block onc ```ruby def new @person = Person.new - 2.times { @person.addresses.build} + 2.times { @person.addresses.build } end ``` diff --git a/guides/source/i18n.md b/guides/source/i18n.md index e6aa6181cc..2b545e6b82 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -977,7 +977,7 @@ en: ``` NOTE: In order to use this helper, you need to install [DynamicForm](https://github.com/joelmoss/dynamic_form) -gem by adding this line to your Gemfile: `gem 'dynamic_form'`. +gem by adding this line to your `Gemfile`: `gem 'dynamic_form'`. ### Translations for Action Mailer E-Mail Subjects diff --git a/guides/source/initialization.md b/guides/source/initialization.md index 1541ea38cd..c4f1df487b 100644 --- a/guides/source/initialization.md +++ b/guides/source/initialization.md @@ -93,7 +93,7 @@ require 'bundler/setup' # Set up gems listed in the Gemfile. In a standard Rails application, there's a `Gemfile` which declares all dependencies of the application. `config/boot.rb` sets -`ENV['BUNDLE_GEMFILE']` to the location of this file. If the Gemfile +`ENV['BUNDLE_GEMFILE']` to the location of this file. If the `Gemfile` exists, then `bundler/setup` is required. The require is used by Bundler to configure the load path for your Gemfile's dependencies. diff --git a/guides/source/routing.md b/guides/source/routing.md index 638f77be13..efc0e32b56 100644 --- a/guides/source/routing.md +++ b/guides/source/routing.md @@ -852,6 +852,49 @@ You can specify unicode character routes directly. For example: get 'こんにちは', to: 'welcome#index' ``` +### Direct routes + +You can create custom URL helpers directly. For example: + +```ruby +direct :homepage do + "http://www.rubyonrails.org" +end + +# >> homepage_url +# => "http://www.rubyonrails.org" +``` + +The return value of the block must be a valid argument for the `url_for` method. So, you can pass a valid string URL, Hash, Array, an Active Model instance, or an Active Model class. + +```ruby +direct :commentable do |model| + [ model, anchor: model.dom_id ] +end + +direct :main do + { controller: 'pages', action: 'index', subdomain: 'www' } +end +``` + +### Using `resolve` + +The `resolve` method allows customizing polymorphic mapping of models. For example: + +``` ruby +resource :basket + +resolve("Basket") { [:basket] } +``` + +``` erb +<%= form_for @basket do |form| %> + <!-- basket form --> +<% end %> +``` + +This will generate the singular URL `/basket` instead of the usual `/baskets/:id`. + Customizing Resourceful Routes ------------------------------ diff --git a/guides/source/security.md b/guides/source/security.md index eeb005b661..916b1e32f8 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -1,7 +1,7 @@ **DO NOT READ THIS FILE ON GITHUB, GUIDES ARE PUBLISHED ON http://guides.rubyonrails.org.** -Ruby on Rails Security Guide -============================ +Securing Rails Applications +=========================== This manual describes common security problems in web applications and how to avoid them with Rails. diff --git a/guides/source/testing.md b/guides/source/testing.md index 8416fd163d..bf310cf2db 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -1,7 +1,7 @@ **DO NOT READ THIS FILE ON GITHUB, GUIDES ARE PUBLISHED ON http://guides.rubyonrails.org.** -A Guide to Testing Rails Applications -===================================== +Testing Rails Applications +========================== This guide covers built-in mechanisms in Rails for testing your application. @@ -319,6 +319,8 @@ specify to make your test failure messages clearer. | `assert_not_includes( collection, obj, [msg] )` | Ensures that `obj` is not in `collection`.| | `assert_in_delta( expected, actual, [delta], [msg] )` | Ensures that the numbers `expected` and `actual` are within `delta` of each other.| | `assert_not_in_delta( expected, actual, [delta], [msg] )` | Ensures that the numbers `expected` and `actual` are not within `delta` of each other.| +| `assert_in_epsilon ( expected, actual, [epsilon], [msg] )` | Ensures that the numbers `expected` and `actual` have a relative error less than `epsilon`.| +| `assert_not_in_epsilon ( expected, actual, [epsilon], [msg] )` | Ensures that the numbers `expected` and `actual` don't have a relative error less than `epsilon`.| | `assert_throws( symbol, [msg] ) { block }` | Ensures that the given block throws the symbol.| | `assert_raises( exception1, exception2, ... ) { block }` | Ensures that the given block raises one of the given exceptions.| | `assert_instance_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class`.| @@ -645,7 +647,7 @@ system tests should live. If you want to change the default settings you can change what the system tests are "driven by". Say you want to change the driver from Selenium to -Poltergeist. First add the `poltergeist` gem to your Gemfile. Then in your +Poltergeist. First add the `poltergeist` gem to your `Gemfile`. Then in your `application_system_test_case.rb` file do the following: ```ruby @@ -671,7 +673,8 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase end ``` -If you want to use a headless browser, you could use Headless Chrome by adding `headless_chrome` in the `:using` argument. +If you want to use a headless browser, you could use Headless Chrome or Headless Firefox by adding +`headless_chrome` or `headless_firefox` in the `:using` argument. ```ruby require "test_helper" diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index 9bc87e4bf0..bb4ef26876 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -1,7 +1,7 @@ **DO NOT READ THIS FILE ON GITHUB, GUIDES ARE PUBLISHED ON http://guides.rubyonrails.org.** -A Guide for Upgrading Ruby on Rails -=================================== +Upgrading Ruby on Rails +======================= This guide provides steps to be followed when you upgrade your applications to a newer version of Ruby on Rails. These steps are also available in individual release guides. @@ -45,7 +45,7 @@ TIP: Ruby 1.8.7 p248 and p249 have marshaling bugs that crash Rails. Ruby Enterp ### The Update Task Rails provides the `app:update` task (`rake rails:update` on 4.2 and earlier). After updating the Rails version -in the Gemfile, run this task. +in the `Gemfile`, run this task. This will help you with the creation of new files and changes of old files in an interactive session. @@ -179,7 +179,7 @@ See [#19034](https://github.com/rails/rails/pull/19034) for more details. `assigns` and `assert_template` have been extracted to the `rails-controller-testing` gem. To continue using these methods in your controller tests, add `gem 'rails-controller-testing'` to -your Gemfile. +your `Gemfile`. If you are using Rspec for testing, please see the extra configuration required in the gem's documentation. @@ -212,7 +212,7 @@ true. `ActiveModel::Serializers::Xml` has been extracted from Rails to the `activemodel-serializers-xml` gem. To continue using XML serialization in your application, add `gem 'activemodel-serializers-xml'` -to your Gemfile. +to your `Gemfile`. ### Removed Support for Legacy `mysql` Database Adapter @@ -278,7 +278,7 @@ You can now just call the dependency once with a wildcard. ### `ActionView::Helpers::RecordTagHelper` moved to external gem (record_tag_helper) -`content_tag_for` and `div_for` have been removed in favor of just using `content_tag`. To continue using the older methods, add the `record_tag_helper` gem to your Gemfile: +`content_tag_for` and `div_for` have been removed in favor of just using `content_tag`. To continue using the older methods, add the `record_tag_helper` gem to your `Gemfile`: ```ruby gem 'record_tag_helper', '~> 1.0' @@ -415,7 +415,7 @@ First, add `gem 'web-console', '~> 2.0'` to the `:development` group in your `Ge ### Responders -`respond_with` and the class-level `respond_to` methods have been extracted to the `responders` gem. To use them, simply add `gem 'responders', '~> 2.0'` to your Gemfile. Calls to `respond_with` and `respond_to` (again, at the class level) will no longer work without having included the `responders` gem in your dependencies: +`respond_with` and the class-level `respond_to` methods have been extracted to the `responders` gem. To use them, simply add `gem 'responders', '~> 2.0'` to your `Gemfile`. Calls to `respond_with` and `respond_to` (again, at the class level) will no longer work without having included the `responders` gem in your dependencies: ```ruby # app/controllers/users_controller.rb @@ -559,7 +559,7 @@ Read the [gem's readme](https://github.com/rails/rails-html-sanitizer) for more The documentation for `PermitScrubber` and `TargetScrubber` explains how you can gain complete control over when and how elements should be stripped. -If your application needs to use the old sanitizer implementation, include `rails-deprecated_sanitizer` in your Gemfile: +If your application needs to use the old sanitizer implementation, include `rails-deprecated_sanitizer` in your `Gemfile`: ```ruby gem 'rails-deprecated_sanitizer' @@ -617,7 +617,7 @@ migration DSL counterpart. The migration procedure is as follows: -1. remove `gem "foreigner"` from the Gemfile. +1. remove `gem "foreigner"` from the `Gemfile`. 2. run `bundle install`. 3. run `bin/rake db:schema:dump`. 4. make sure that `db/schema.rb` contains every foreign key definition with @@ -769,7 +769,7 @@ and has been removed from Rails. If your application currently depends on MultiJSON directly, you have a few options: -1. Add 'multi_json' to your Gemfile. Note that this might cease to work in the future +1. Add 'multi_json' to your `Gemfile`. Note that this might cease to work in the future 2. Migrate away from MultiJSON by using `obj.to_json`, and `JSON.parse(str)` instead. @@ -810,7 +810,7 @@ part of the rewrite, the following features have been removed from the encoder: If your application depends on one of these features, you can get them back by adding the [`activesupport-json_encoder`](https://github.com/rails/activesupport-json_encoder) -gem to your Gemfile. +gem to your `Gemfile`. #### JSON representation of Time objects @@ -1135,7 +1135,7 @@ full support for the last few changes in the specification. ### Gemfile -Rails 4.0 removed the `assets` group from Gemfile. You'd need to remove that +Rails 4.0 removed the `assets` group from `Gemfile`. You'd need to remove that line from your `Gemfile` when upgrading. You should also update your application file (in `config/application.rb`): @@ -1147,7 +1147,7 @@ Bundler.require(*Rails.groups) ### vendor/plugins -Rails 4.0 no longer supports loading plugins from `vendor/plugins`. You must replace any plugins by extracting them to gems and adding them to your Gemfile. If you choose not to make them gems, you can move them into, say, `lib/my_plugin/*` and add an appropriate initializer in `config/initializers/my_plugin.rb`. +Rails 4.0 no longer supports loading plugins from `vendor/plugins`. You must replace any plugins by extracting them to gems and adding them to your `Gemfile`. If you choose not to make them gems, you can move them into, say, `lib/my_plugin/*` and add an appropriate initializer in `config/initializers/my_plugin.rb`. ### Active Record @@ -1214,7 +1214,7 @@ end ### Active Resource -Rails 4.0 extracted Active Resource to its own gem. If you still need the feature you can add the [Active Resource gem](https://github.com/rails/activeresource) in your Gemfile. +Rails 4.0 extracted Active Resource to its own gem. If you still need the feature you can add the [Active Resource gem](https://github.com/rails/activeresource) in your `Gemfile`. ### Active Model @@ -1414,7 +1414,7 @@ config.active_record.mass_assignment_sanitizer = :strict ### vendor/plugins -Rails 3.2 deprecates `vendor/plugins` and Rails 4.0 will remove them completely. While it's not strictly necessary as part of a Rails 3.2 upgrade, you can start replacing any plugins by extracting them to gems and adding them to your Gemfile. If you choose not to make them gems, you can move them into, say, `lib/my_plugin/*` and add an appropriate initializer in `config/initializers/my_plugin.rb`. +Rails 3.2 deprecates `vendor/plugins` and Rails 4.0 will remove them completely. While it's not strictly necessary as part of a Rails 3.2 upgrade, you can start replacing any plugins by extracting them to gems and adding them to your `Gemfile`. If you choose not to make them gems, you can move them into, say, `lib/my_plugin/*` and add an appropriate initializer in `config/initializers/my_plugin.rb`. ### Active Record diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index 86746a5ae0..c3dff1772c 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -492,7 +492,7 @@ replace the entire `<body>` of the page with the `<body>` of the response. It will then use PushState to change the URL to the correct one, preserving refresh semantics and giving you pretty URLs. -The only thing you have to do to enable Turbolinks is have it in your Gemfile, +The only thing you have to do to enable Turbolinks is have it in your `Gemfile`, and put `//= require turbolinks` in your JavaScript manifest, which is usually `app/assets/javascripts/application.js`. diff --git a/railties/lib/rails/commands/secrets/secrets_command.rb b/railties/lib/rails/commands/secrets/secrets_command.rb index 73a88767e2..a36ccf314c 100644 --- a/railties/lib/rails/commands/secrets/secrets_command.rb +++ b/railties/lib/rails/commands/secrets/secrets_command.rb @@ -56,7 +56,7 @@ module Rails private def deprecate_in_favor_of_credentials_and_exit say "Encrypted secrets is deprecated in favor of credentials. Run:" - say "bin/rails credentials --help" + say "bin/rails credentials:help" exit 1 end diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt index 61026f5182..e3ed3e7c11 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt @@ -41,13 +41,6 @@ gem 'bootsnap', '>= 1.1.0', require: false group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] - <%- if depends_on_system_test? -%> - # Adds support for Capybara system testing and selenium driver - gem 'capybara', '~> 2.15' - gem 'selenium-webdriver' - # Easy installation and use of chromedriver to run system tests with Chrome - gem 'chromedriver-helper' - <%- end -%> end group :development do @@ -70,6 +63,16 @@ group :development do <% end -%> <% end -%> end + +<%- if depends_on_system_test? -%> +group :test do + # Adds support for Capybara system testing and selenium driver + gem 'capybara', '~> 2.15' + gem 'selenium-webdriver' + # Easy installation and use of chromedriver to run system tests with Chrome + gem 'chromedriver-helper' +end +<%- end -%> <% end -%> # Windows does not include zoneinfo files, so bundle the tzinfo-data gem 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 b383228dc0..961e167d24 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 @@ -46,6 +46,9 @@ Rails.application.configure do # Raise an error on page load if there are pending migrations. config.active_record.migration_error = :page_load + # Highlight code that triggered database queries in logs. + config.active_record.verbose_query_logs = true + <%- end -%> <%- unless options.skip_sprockets? -%> # Debug mode disables concatenation and preprocessing of assets. diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_2.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_2.rb.tt index 25dcddb27a..f630d9985a 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_2.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_2.rb.tt @@ -25,3 +25,6 @@ # Store boolean values are in sqlite3 databases as 1 and 0 instead of 't' and # 'f' after migrating old data. # Rails.application.config.active_record.sqlite3.represent_boolean_as_integer = true + +# Highlight code that triggered database queries in logs. +Rails.application.config.active_record.verbose_query_logs = Rails.env.development? diff --git a/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt index 1e19380dcb..a5eccf816b 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt @@ -26,31 +26,9 @@ environment ENV.fetch("RAILS_ENV") { "development" } # Use the `preload_app!` method when specifying a `workers` number. # This directive tells Puma to first boot the application and load code # before forking the application. This takes advantage of Copy On Write -# process behavior so workers use less memory. If you use this option -# you need to make sure to reconnect any threads in the `on_worker_boot` -# block. +# process behavior so workers use less memory. # # preload_app! -# If you are preloading your application and using Active Record, it's -# recommended that you close any connections to the database before workers -# are forked to prevent connection leakage. -# -# before_fork do -# ActiveRecord::Base.connection_pool.disconnect! if defined?(ActiveRecord) -# end - -# The code in the `on_worker_boot` will be called if you are using -# clustered mode by specifying a number of `workers`. After each worker -# process is booted, this block will be run. If you are using the `preload_app!` -# option, you will want to use this block to reconnect to any threads -# or connections that may have been created at application boot, as Ruby -# cannot share connections between processes. -# -# on_worker_boot do -# ActiveRecord::Base.establish_connection if defined?(ActiveRecord) -# end -# - # Allow puma to be restarted by `rails restart` command. plugin :tmp_restart diff --git a/railties/lib/rails/generators/rails/app/templates/test/test_helper.rb.tt b/railties/lib/rails/generators/rails/app/templates/test/test_helper.rb.tt index 6ad1f11781..52d68cc77c 100644 --- a/railties/lib/rails/generators/rails/app/templates/test/test_helper.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/test/test_helper.rb.tt @@ -1,3 +1,4 @@ +ENV['RAILS_ENV'] ||= 'test' require_relative '../config/environment' require 'rails/test_help' diff --git a/railties/lib/rails/generators/rails/credentials/credentials_generator.rb b/railties/lib/rails/generators/rails/credentials/credentials_generator.rb index 067479c672..01a5b502f9 100644 --- a/railties/lib/rails/generators/rails/credentials/credentials_generator.rb +++ b/railties/lib/rails/generators/rails/credentials/credentials_generator.rb @@ -8,7 +8,7 @@ module Rails module Generators class CredentialsGenerator < Base def add_credentials_file - unless credentials.exist? + unless credentials.content_path.exist? template = credentials_template say "Adding #{credentials.content_path} to store encrypted credentials." @@ -26,7 +26,9 @@ module Rails end def add_credentials_file_silently(template = nil) - credentials.write(credentials_template) + unless credentials.content_path.exist? + credentials.write(credentials_template) + end end private diff --git a/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb.tt b/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb.tt index 7fa9973931..755d19ef5d 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb.tt +++ b/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb.tt @@ -1,3 +1,6 @@ +# Configure Rails Environment +ENV["RAILS_ENV"] = "test" + require_relative "<%= File.join('..', options[:dummy_path], 'config/environment') -%>" <% unless options[:skip_active_record] -%> ActiveRecord::Migrator.migrations_paths = [File.expand_path("../<%= options[:dummy_path] -%>/db/migrate", __dir__)] diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb index 732c5c1e1f..76c28ac85e 100644 --- a/railties/lib/rails/test_help.rb +++ b/railties/lib/rails/test_help.rb @@ -29,10 +29,6 @@ if defined?(ActiveRecord::Base) end ActionDispatch::IntegrationTest.fixture_path = ActiveSupport::TestCase.fixture_path - - def create_fixtures(*fixture_set_names, &block) - FixtureSet.create_fixtures(ActiveSupport::TestCase.fixture_path, fixture_set_names, {}, &block) - end end # :enddoc: diff --git a/railties/lib/rails/test_unit/runner.rb b/railties/lib/rails/test_unit/runner.rb index 5c2f6451e2..de5744c662 100644 --- a/railties/lib/rails/test_unit/runner.rb +++ b/railties/lib/rails/test_unit/runner.rb @@ -13,7 +13,7 @@ module Rails class << self def attach_before_load_options(opts) opts.on("--warnings", "-w", "Run with Ruby warnings enabled") {} - opts.on("--environment", "-e", "Run tests in the ENV environment") {} + opts.on("-e", "--environment ENV", "Run tests in the ENV environment") {} end def parse_options(argv) diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index edb6190ed0..d28f7ffc7f 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1317,7 +1317,7 @@ module ApplicationTests assert_equal 200, last_response.status end - test "config.action_controller.action_on_unpermitted_parameters is :log by default on development" do + test "config.action_controller.action_on_unpermitted_parameters is :log by default in development" do app "development" force_lazy_load_hooks { ActionController::Base } @@ -1326,7 +1326,7 @@ module ApplicationTests assert_equal :log, ActionController::Parameters.action_on_unpermitted_parameters end - test "config.action_controller.action_on_unpermitted_parameters is :log by default on test" do + test "config.action_controller.action_on_unpermitted_parameters is :log by default in test" do app "test" force_lazy_load_hooks { ActionController::Base } @@ -1335,7 +1335,7 @@ module ApplicationTests assert_equal :log, ActionController::Parameters.action_on_unpermitted_parameters end - test "config.action_controller.action_on_unpermitted_parameters is false by default on production" do + test "config.action_controller.action_on_unpermitted_parameters is false by default in production" do app "production" force_lazy_load_hooks { ActionController::Base } @@ -1482,12 +1482,18 @@ module ApplicationTests assert_not ActiveRecord::Base.dump_schema_after_migration end - test "config.active_record.dump_schema_after_migration is true by default on development" do + test "config.active_record.dump_schema_after_migration is true by default in development" do app "development" assert ActiveRecord::Base.dump_schema_after_migration end + test "config.active_record.verbose_query_logs is false by default in development" do + app "development" + + assert_not ActiveRecord::Base.verbose_query_logs + end + test "config.annotations wrapping SourceAnnotationExtractor::Annotation class" do make_basic_app do |application| application.config.annotations.register_extensions("coffee") do |tag| diff --git a/railties/test/application/initializers/notifications_test.rb b/railties/test/application/initializers/notifications_test.rb index 23b20d578c..c65c955734 100644 --- a/railties/test/application/initializers/notifications_test.rb +++ b/railties/test/application/initializers/notifications_test.rb @@ -32,6 +32,7 @@ module ApplicationTests logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new ActiveRecord::Base.logger = logger + ActiveRecord::Base.verbose_query_logs = false # Mimic Active Record notifications instrument "sql.active_record", name: "SQL", sql: "SHOW tables" diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb index 0235210fdd..5b4c42c189 100644 --- a/railties/test/application/rake/dbs_test.rb +++ b/railties/test/application/rake/dbs_test.rb @@ -98,6 +98,20 @@ module ApplicationTests end end + test "db:create works when schema cache exists and database does not exist" do + use_postgresql + + begin + rails %w(db:create db:migrate db:schema:cache:dump) + + rails "db:drop" + rails "db:create" + assert_equal 0, $?.exitstatus + ensure + rails "db:drop" rescue nil + end + end + test "db:drop failure because database does not exist" do output = rails("db:drop:_unsafe", "--trace") assert_match(/does not exist/, output) @@ -189,7 +203,7 @@ module ApplicationTests rails "environment", "db:drop", "db:structure:load" assert_match expected_database, ActiveRecord::Base.connection_config[:database] require "#{app_path}/app/models/book" - #if structure is not loaded correctly, exception would be raised + # if structure is not loaded correctly, exception would be raised assert_equal 0, Book.count end end @@ -284,7 +298,7 @@ module ApplicationTests ActiveRecord::Base.configurations = Rails.application.config.database_configuration ActiveRecord::Base.establish_connection :test require "#{app_path}/app/models/book" - #if structure is not loaded correctly, exception would be raised + # if structure is not loaded correctly, exception would be raised assert_equal 0, Book.count assert_match ActiveRecord::Base.configurations["test"]["database"], ActiveRecord::Base.connection_config[:database] diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index bf89098645..5a6404bd0a 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -132,8 +132,14 @@ module ApplicationTests output = rails("routes") assert_equal <<-MESSAGE.strip_heredoc, output - Prefix Verb URI Pattern Controller#Action - cart GET /cart(.:format) cart#show + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show + rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show + rails_blob_preview GET /rails/active_storage/previews/:signed_blob_id/:variation_key/*filename(.:format) active_storage/previews#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show + update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update + rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create MESSAGE end @@ -168,14 +174,19 @@ module ApplicationTests output = rails("routes", "-g", "show") assert_equal <<-MESSAGE.strip_heredoc, output - Prefix Verb URI Pattern Controller#Action - cart GET /cart(.:format) cart#show + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show + rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show + rails_blob_preview GET /rails/active_storage/previews/:signed_blob_id/:variation_key/*filename(.:format) active_storage/previews#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show MESSAGE output = rails("routes", "-g", "POST") assert_equal <<-MESSAGE.strip_heredoc, output - Prefix Verb URI Pattern Controller#Action - POST /cart(.:format) cart#create + Prefix Verb URI Pattern Controller#Action + POST /cart(.:format) cart#create + rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create MESSAGE output = rails("routes", "-g", "basketballs") @@ -232,11 +243,13 @@ module ApplicationTests RUBY assert_equal <<-MESSAGE.strip_heredoc, rails("routes") - You don't have any routes defined! - - Please add some routes in config/routes.rb. - - For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html. + Prefix Verb URI Pattern Controller#Action + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show + rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show + rails_blob_preview GET /rails/active_storage/previews/:signed_blob_id/:variation_key/*filename(.:format) active_storage/previews#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show + update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update + rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create MESSAGE end @@ -250,8 +263,14 @@ module ApplicationTests output = Dir.chdir(app_path) { `bin/rake --rakefile Rakefile routes` } assert_equal <<-MESSAGE.strip_heredoc, output - Prefix Verb URI Pattern Controller#Action - cart GET /cart(.:format) cart#show + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show + rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show + rails_blob_preview GET /rails/active_storage/previews/:signed_blob_id/:variation_key/*filename(.:format) active_storage/previews#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show + update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update + rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create MESSAGE end diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index e92a0466dd..a01325fdb8 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -569,6 +569,40 @@ module ApplicationTests assert_match "AccountTest#test_truth", output, "passing TEST= should run selected test" end + def test_running_with_ruby_gets_test_env_by_default + # Subshells inherit `ENV`, so we need to ensure `RAILS_ENV` is set to + # nil before we run the tests in the test app. + re, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], nil + + file = create_test_for_env("test") + results = Dir.chdir(app_path) { + `ruby -Ilib:test #{file}`.each_line.map { |line| JSON.parse line } + } + assert_equal 1, results.length + failures = results.first["failures"] + flunk(failures.first) unless failures.empty? + + ensure + ENV["RAILS_ENV"] = re + end + + def test_running_with_ruby_can_set_env_via_cmdline + # Subshells inherit `ENV`, so we need to ensure `RAILS_ENV` is set to + # nil before we run the tests in the test app. + re, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], nil + + file = create_test_for_env("development") + results = Dir.chdir(app_path) { + `RAILS_ENV=development ruby -Ilib:test #{file}`.each_line.map { |line| JSON.parse line } + } + assert_equal 1, results.length + failures = results.first["failures"] + flunk(failures.first) unless failures.empty? + + ensure + ENV["RAILS_ENV"] = re + end + def test_rake_passes_multiple_TESTOPTS_to_minitest create_test_file :models, "account" output = Dir.chdir(app_path) { `bin/rake test TESTOPTS='-v --seed=1234'` } @@ -727,6 +761,45 @@ module ApplicationTests app_file "db/schema.rb", "" end + def create_test_for_env(env) + app_file "test/models/environment_test.rb", <<-RUBY + require 'test_helper' + class JSONReporter < Minitest::AbstractReporter + def record(result) + puts JSON.dump(klass: result.class.name, + name: result.name, + failures: result.failures, + assertions: result.assertions, + time: result.time) + end + end + + def Minitest.plugin_json_reporter_init(opts) + Minitest.reporter.reporters.clear + Minitest.reporter.reporters << JSONReporter.new + end + + Minitest.extensions << "rails" + Minitest.extensions << "json_reporter" + + # Minitest uses RubyGems to find plugins, and since RubyGems + # doesn't know about the Rails installation we're pointing at, + # Minitest won't require the Rails minitest plugin when we run + # these integration tests. So we have to manually require the + # Minitest plugin here. + require 'minitest/rails_plugin' + + class EnvironmentTest < ActiveSupport::TestCase + def test_environment + test_db = ActiveRecord::Base.configurations[#{env.dump}]["database"] + db_file = ActiveRecord::Base.connection_config[:database] + assert_match(test_db, db_file) + assert_equal #{env.dump}, ENV["RAILS_ENV"] + end + end + RUBY + end + def create_test_file(path = :unit, name = "test", pass: true) app_file "test/#{path}/#{name}_test.rb", <<-RUBY require 'test_helper' diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb index 4ef827fcf1..f1bb1ef08a 100644 --- a/railties/test/commands/credentials_test.rb +++ b/railties/test/commands/credentials_test.rb @@ -39,6 +39,14 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase end end + test "edit command does not overwrite by default if credentials already exists" do + run_edit_command(editor: "eval echo api_key: abc >") + assert_match(/api_key: abc/, run_show_command) + + run_edit_command + assert_match(/api_key: abc/, run_show_command) + end + private def run_edit_command(editor: "cat") switch_env("EDITOR", editor) do diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 774fd0f315..96803db838 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -310,7 +310,7 @@ class AppGeneratorTest < Rails::Generators::TestCase 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." + assert_equal 1, @binstub_called, "active_storage:install expected to be called once, but was called #{@binstub_called} times" end end @@ -743,6 +743,41 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_webpack_option + command_check = -> command, *_ do + @called ||= 0 + if command == "webpacker:install" + @called += 1 + assert_equal 1, @called, "webpacker:install expected to be called once, but was called #{@called} times." + end + end + + generator([destination_root], webpack: "webpack").stub(:rails_command, command_check) do + quietly { generator.invoke_all } + end + + assert_gem "webpacker" + end + + def test_webpack_option_with_js_framework + command_check = -> command, *_ do + case command + when "webpacker:install" + @webpacker ||= 0 + @webpacker += 1 + assert_equal 1, @webpacker, "webpacker:install expected to be called once, but was called #{@webpacker} times." + when "webpacker:install:react" + @react ||= 0 + @react += 1 + assert_equal 1, @react, "webpacker:install:react expected to be called once, but was called #{@react} times." + end + end + + generator([destination_root], webpack: "react").stub(:rails_command, command_check) do + quietly { generator.invoke_all } + end + end + def test_generator_if_skip_turbolinks_is_given run_generator [destination_root, "--skip-turbolinks"] diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 7522237a38..96c6f21395 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -358,10 +358,12 @@ module TestHelpers end def app_file(path, contents, mode = "w") - FileUtils.mkdir_p File.dirname("#{app_path}/#{path}") - File.open("#{app_path}/#{path}", mode) do |f| + file_name = "#{app_path}/#{path}" + FileUtils.mkdir_p File.dirname(file_name) + File.open(file_name, mode) do |f| f.puts contents end + file_name end def remove_file(path) @@ -381,6 +383,21 @@ module TestHelpers $:.reject! { |path| path =~ %r'/(#{to_remove.join('|')})/' } end + + def use_postgresql + File.open("#{app_path}/config/database.yml", "w") do |f| + f.puts <<-YAML + default: &default + adapter: postgresql + pool: 5 + database: railties_test + development: + <<: *default + test: + <<: *default + YAML + end + end end end diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index fc710feb63..4a861bff55 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -980,14 +980,14 @@ YAML boot_rails app_generators = Rails.application.config.generators.options[:rails] - assert_equal :mongoid , app_generators[:orm] - assert_equal :liquid , app_generators[:template_engine] + assert_equal :mongoid, app_generators[:orm] + assert_equal :liquid, app_generators[:template_engine] assert_equal :test_unit, app_generators[:test_framework] generators = Bukkits::Engine.config.generators.options[:rails] assert_equal :data_mapper, generators[:orm] - assert_equal :haml , generators[:template_engine] - assert_equal :rspec , generators[:test_framework] + assert_equal :haml, generators[:template_engine] + assert_equal :rspec, generators[:test_framework] end test "engine should get default generators with ability to overwrite them" do @@ -1003,10 +1003,10 @@ YAML generators = Bukkits::Engine.config.generators.options[:rails] assert_equal :active_record, generators[:orm] - assert_equal :rspec , generators[:test_framework] + assert_equal :rspec, generators[:test_framework] app_generators = Rails.application.config.generators.options[:rails] - assert_equal :test_unit , app_generators[:test_framework] + assert_equal :test_unit, app_generators[:test_framework] end test "do not create table_name_prefix method if it already exists" do |