diff options
84 files changed, 1382 insertions, 358 deletions
@@ -14,7 +14,7 @@ gem "rake", ">= 11.1" # be loaded after loading the test library. gem "mocha", "~> 0.14", require: false -gem "capybara", "~> 2.7.0" +gem "capybara", "~> 2.13.0" gem "rack-cache", "~> 1.2" gem "jquery-rails" diff --git a/Gemfile.lock b/Gemfile.lock index 68c69f75a0..580333f6e6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -120,7 +120,7 @@ GEM bunny (2.6.2) amq-protocol (>= 2.0.1) byebug (9.0.6) - capybara (2.7.1) + capybara (2.13.0) addressable mime-types (>= 1.16) nokogiri (>= 1.3.3) @@ -377,7 +377,7 @@ DEPENDENCIES blade blade-sauce_labs_plugin byebug - capybara (~> 2.7.0) + capybara (~> 2.13.0) coffee-rails dalli (>= 2.2.1) delayed_job diff --git a/actionmailer/test/message_delivery_test.rb b/actionmailer/test/message_delivery_test.rb index f4c4f43bdc..c0683be94d 100644 --- a/actionmailer/test/message_delivery_test.rb +++ b/actionmailer/test/message_delivery_test.rb @@ -76,14 +76,14 @@ class MessageDeliveryTest < ActiveSupport::TestCase test "should enqueue a delivery with a delay" do travel_to Time.new(2004, 11, 24, 01, 04, 44) do - assert_performed_with(job: ActionMailer::DeliveryJob, at: Time.current.to_f + 600, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do - @mail.deliver_later wait: 600.seconds + assert_performed_with(job: ActionMailer::DeliveryJob, at: Time.current + 10.minutes, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do + @mail.deliver_later wait: 10.minutes end end end test "should enqueue a delivery at a specific time" do - later_time = Time.now.to_f + 3600 + later_time = Time.current + 1.hour assert_performed_with(job: ActionMailer::DeliveryJob, at: later_time, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do @mail.deliver_later wait_until: later_time end diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index 984ded1ff5..e89ea8b21d 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -40,7 +40,7 @@ module ActionDispatch # # Example usage: # - # edit_polymorphic_path(@post) # => "/posts/1/edit" + # edit_polymorphic_path(@post) # => "/posts/1/edit" # polymorphic_path(@post, format: :pdf) # => "/posts/1.pdf" # # == Usage with mounted engines @@ -104,7 +104,7 @@ module ActionDispatch end if mapping = polymorphic_mapping(record_or_hash_or_array) - return mapping.call(self, [record_or_hash_or_array, options]) + return mapping.call(self, [record_or_hash_or_array, options], false) end opts = options.dup @@ -128,7 +128,7 @@ module ActionDispatch end if mapping = polymorphic_mapping(record_or_hash_or_array) - return mapping.call(self, [record_or_hash_or_array, options], only_path: true) + return mapping.call(self, [record_or_hash_or_array, options], true) end opts = options.dup @@ -273,7 +273,7 @@ module ActionDispatch def handle_model_call(target, record) if mapping = polymorphic_mapping(target, record) - mapping.call(target, [record], only_path: suffix == "path") + mapping.call(target, [record], suffix == "path") else method, args = handle_model(record) target.send(method, *args) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index c4719f8a71..7034eff36d 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -164,13 +164,13 @@ module ActionDispatch @path_helpers_module.module_eval do define_method(:"#{name}_path") do |*args| - helper.call(self, args, only_path: true) + helper.call(self, args, true) end end @url_helpers_module.module_eval do define_method(:"#{name}_url") do |*args| - helper.call(self, args) + helper.call(self, args, false) end end end @@ -509,6 +509,14 @@ module ActionDispatch @_proxy.url_for(options) end + def full_url_for(options) + @_proxy.full_url_for(options) + end + + def route_for(name, *args) + @_proxy.route_for(name, *args) + end + def optimize_routes_generation? @_proxy.optimize_routes_generation? end @@ -613,26 +621,14 @@ module ActionDispatch @block = block end - def call(t, args, outer_options = {}) + def call(t, args, only_path = false) options = args.extract_options! - url_options = eval_block(t, args, options) - - case url_options - when String - t.url_for(url_options) - when Hash - t.url_for(url_options.merge(outer_options)) - when ActionController::Parameters - if url_options.permitted? - t.url_for(url_options.to_h.merge(outer_options)) - else - raise ArgumentError, "Generating a URL from non sanitized request parameters is insecure!" - end - when Array - opts = url_options.extract_options! - t.url_for(url_options.push(opts.merge(outer_options))) + url = t.full_url_for(eval_block(t, args, options)) + + if only_path + "/" + url.partition(%r{(?<!/)/(?!/)}).last else - t.url_for([url_options, outer_options]) + url end end @@ -860,8 +856,7 @@ module ActionDispatch params[key] = URI.parser.unescape(value) end end - old_params = req.path_parameters - req.path_parameters = old_params.merge params + req.path_parameters = params app = route.app if app.matches?(req) && app.dispatcher? begin diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 3e564f13d8..4a23e1c8b6 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -164,6 +164,10 @@ module ActionDispatch # implicitly used by +url_for+ can always be overwritten like shown on the # last +url_for+ calls. def url_for(options = nil) + full_url_for(options) + end + + def full_url_for(options = nil) # :nodoc: case options when nil _routes.url_for(url_options.symbolize_keys) @@ -192,6 +196,10 @@ module ActionDispatch end end + def route_for(name, *args) # :nodoc: + public_send(:"#{name}_url", *args) + end + protected def optimize_routes_generation? diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb index 1bf47d2556..f908607e50 100644 --- a/actionpack/lib/action_dispatch/system_test_case.rb +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -1,4 +1,5 @@ require "capybara/dsl" +require "capybara/minitest" require "action_controller" require "action_dispatch/system_testing/driver" require "action_dispatch/system_testing/server" @@ -80,6 +81,7 @@ module ActionDispatch # tests as long as you include the required gems and files. class SystemTestCase < IntegrationTest include Capybara::DSL + include Capybara::Minitest::Assertions include SystemTesting::TestHelpers::SetupAndTeardown include SystemTesting::TestHelpers::ScreenshotHelper @@ -110,8 +112,8 @@ module ActionDispatch # driven_by :selenium, using: :firefox # # driven_by :selenium, screen_size: [800, 800] - def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400]) - @driver = SystemTesting::Driver.new(driver, using: using, screen_size: screen_size) + def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400], options: {}) + @driver = SystemTesting::Driver.new(driver, using: using, screen_size: screen_size, options: options) end # Returns the driver object for the initialized system test diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb index 72d132d64f..5cf17883f7 100644 --- a/actionpack/lib/action_dispatch/system_testing/driver.rb +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -5,6 +5,7 @@ module ActionDispatch @name = name @browser = options[:using] @screen_size = options[:screen_size] + @options = options[:options] end def use @@ -19,7 +20,7 @@ module ActionDispatch def register Capybara.register_driver @name do |app| - Capybara::Selenium::Driver.new(app, browser: @browser).tap do |driver| + Capybara::Selenium::Driver.new(app, { browser: @browser }.merge(@options)).tap do |driver| driver.browser.manage.window.size = Selenium::WebDriver::Dimension.new(*@screen_size) end end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index a3430e210e..b46a97413a 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -192,11 +192,10 @@ module ActionDispatch # HTTP methods in integration tests. +#process+ is only required when using a # request method that doesn't have a method defined in the integration tests. # - # This method returns a Response object, which one can use to - # inspect the details of the response. Furthermore, if this method was - # called from an ActionDispatch::IntegrationTest object, then that - # object's <tt>@response</tt> instance variable will point to the same - # response object. + # This method returns the response status, after performing the request. + # Furthermore, if this method was called from an ActionDispatch::IntegrationTest object, + # then that object's <tt>@response</tt> instance variable will point to a Response object + # which one can use to inspect the details of the response. # # Example: # process :get, '/author', params: { since: 201501011400 } diff --git a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb index f85b989892..cb5ca5888b 100644 --- a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb +++ b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb @@ -4,18 +4,23 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest class Linkable attr_reader :id + def self.name + super.demodulize + end + def initialize(id) @id = id end def linkable_type - self.class.name.demodulize.underscore + self.class.name.underscore end end class Category < Linkable; end class Collection < Linkable; end class Product < Linkable; end + class Manufacturer < Linkable; end class Model extend ActiveModel::Naming @@ -79,7 +84,7 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest get "/media/:id", to: "media#show", as: :media get "/pages/:id", to: "pages#show", as: :page - resources :categories, :collections, :products + resources :categories, :collections, :products, :manufacturers namespace :admin do get "/dashboard", to: "dashboard#index" @@ -89,6 +94,7 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest direct("string") { "http://www.rubyonrails.org" } direct(:helper) { basket_url } direct(:linkable) { |linkable| [:"#{linkable.linkable_type}", { id: linkable.id }] } + direct(:nested) { |linkable| route_for(:linkable, linkable) } direct(:params) { |params| params } direct(:symbol) { :basket } direct(:hash) { { controller: "basket", action: "show" } } @@ -102,6 +108,7 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest resolve("Article") { |article| [:post, { id: article.id }] } resolve("Basket") { |basket| [:basket] } + resolve("Manufacturer") { |manufacturer| route_for(:linkable, manufacturer) } resolve("User", anchor: "details") { |user, options| [:profile, options] } resolve("Video") { |video| [:media, { id: video.id }] } resolve(%w[Page CategoryPage ProductPage]) { |page| [:page, { id: page.id }] } @@ -119,6 +126,7 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest @category = Category.new("1") @collection = Collection.new("2") @product = Product.new("3") + @manufacturer = Manufacturer.new("apple") @basket = Basket.new @user = User.new @video = Video.new("4") @@ -136,14 +144,14 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest end def test_direct_paths - assert_equal "http://www.rubyonrails.org", website_path - assert_equal "http://www.rubyonrails.org", Routes.url_helpers.website_path + assert_equal "/", website_path + assert_equal "/", Routes.url_helpers.website_path - assert_equal "http://www.rubyonrails.org", string_path - assert_equal "http://www.rubyonrails.org", Routes.url_helpers.string_path + assert_equal "/", string_path + assert_equal "/", Routes.url_helpers.string_path - assert_equal "http://www.example.com/basket", helper_url - assert_equal "http://www.example.com/basket", Routes.url_helpers.helper_url + assert_equal "/basket", helper_path + assert_equal "/basket", Routes.url_helpers.helper_path assert_equal "/categories/1", linkable_path(@category) assert_equal "/categories/1", Routes.url_helpers.linkable_path(@category) @@ -152,6 +160,9 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest assert_equal "/products/3", linkable_path(@product) assert_equal "/products/3", Routes.url_helpers.linkable_path(@product) + assert_equal "/categories/1", nested_path(@category) + assert_equal "/categories/1", Routes.url_helpers.nested_path(@category) + assert_equal "/", params_path(@safe_params) assert_equal "/", Routes.url_helpers.params_path(@safe_params) assert_raises(ArgumentError) { params_path(@unsafe_params) } @@ -192,6 +203,9 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest assert_equal "http://www.example.com/products/3", linkable_url(@product) assert_equal "http://www.example.com/products/3", Routes.url_helpers.linkable_url(@product) + assert_equal "http://www.example.com/categories/1", nested_url(@category) + assert_equal "http://www.example.com/categories/1", Routes.url_helpers.nested_url(@category) + assert_equal "http://www.example.com/", params_url(@safe_params) assert_equal "http://www.example.com/", Routes.url_helpers.params_url(@safe_params) assert_raises(ArgumentError) { params_url(@unsafe_params) } @@ -244,6 +258,9 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest assert_equal "/pages/8", polymorphic_path(@product_page) assert_equal "/pages/8", Routes.url_helpers.polymorphic_path(@product_page) assert_equal "/pages/8", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @product_page) + + assert_equal "/manufacturers/apple", polymorphic_path(@manufacturer) + assert_equal "/manufacturers/apple", Routes.url_helpers.polymorphic_path(@manufacturer) end def test_resolve_urls @@ -277,6 +294,9 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest assert_equal "http://www.example.com/pages/8", polymorphic_url(@product_page) assert_equal "http://www.example.com/pages/8", Routes.url_helpers.polymorphic_url(@product_page) assert_equal "http://www.example.com/pages/8", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @product_page) + + assert_equal "http://www.example.com/manufacturers/apple", polymorphic_url(@manufacturer) + assert_equal "http://www.example.com/manufacturers/apple", Routes.url_helpers.polymorphic_url(@manufacturer) end def test_defining_direct_inside_a_scope_raises_runtime_error diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index d563df91df..64818e6ca1 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4962,3 +4962,64 @@ class FlashRedirectTest < ActionDispatch::IntegrationTest assert_equal "bar", response.body end end + +class TestRecognizePath < ActionDispatch::IntegrationTest + class PageConstraint + attr_reader :key, :pattern + + def initialize(key, pattern) + @key = key + @pattern = pattern + end + + def matches?(request) + request.path_parameters[key] =~ pattern + end + end + + stub_controllers do |routes| + Routes = routes + routes.draw do + get "/hash/:foo", to: "pages#show", constraints: { foo: /foo/ } + get "/hash/:bar", to: "pages#show", constraints: { bar: /bar/ } + + get "/proc/:foo", to: "pages#show", constraints: proc { |r| r.path_parameters[:foo] =~ /foo/ } + get "/proc/:bar", to: "pages#show", constraints: proc { |r| r.path_parameters[:bar] =~ /bar/ } + + get "/class/:foo", to: "pages#show", constraints: PageConstraint.new(:foo, /foo/) + get "/class/:bar", to: "pages#show", constraints: PageConstraint.new(:bar, /bar/) + end + end + + APP = build_app Routes + def app + APP + end + + def test_hash_constraints_dont_leak_between_routes + expected_params = { controller: "pages", action: "show", bar: "bar" } + actual_params = recognize_path("/hash/bar") + + assert_equal expected_params, actual_params + end + + def test_proc_constraints_dont_leak_between_routes + expected_params = { controller: "pages", action: "show", bar: "bar" } + actual_params = recognize_path("/proc/bar") + + assert_equal expected_params, actual_params + end + + def test_class_constraints_dont_leak_between_routes + expected_params = { controller: "pages", action: "show", bar: "bar" } + actual_params = recognize_path("/class/bar") + + assert_equal expected_params, actual_params + end + + private + + def recognize_path(*args) + Routes.recognize_path(*args) + end +end diff --git a/actionpack/test/dispatch/system_testing/driver_test.rb b/actionpack/test/dispatch/system_testing/driver_test.rb index 8f8777b19f..814e1d707b 100644 --- a/actionpack/test/dispatch/system_testing/driver_test.rb +++ b/actionpack/test/dispatch/system_testing/driver_test.rb @@ -8,10 +8,11 @@ class DriverTest < ActiveSupport::TestCase end test "initializing the driver with a browser" do - driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :chrome, screen_size: [1400, 1400]) + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :chrome, screen_size: [1400, 1400], options: { url: "http://example.com/wd/hub" }) assert_equal :selenium, driver.instance_variable_get(:@name) assert_equal :chrome, 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 "selenium? returns false if driver is poltergeist" do diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index e9e4580f96..6e71809385 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -99,6 +99,23 @@ *Peter Schilling*, *Matthew Draper* +* Add `:skip_pipeline` option to several asset tag helpers + + `javascript_include_tag`, `stylesheet_link_tag`, `favicon_link_tag`, + `image_tag` and `audio_tag` now accept a `:skip_pipeline` option which can + be set to true to bypass the asset pipeline and serve the assets from the + public folder. + + *Richard Schneeman* + +* Add `:poster_skip_pipeline` option to the `video_tag` helper + + `video_tag` now accepts a `:poster_skip_pipeline` option which can be used + in combination with the `:poster` option to bypass the asset pipeline and + serve the poster image for the video from the public folder. + + *Richard Schneeman* + * Show cache hits and misses when rendering partials. Partials using the `cache` helper will show whether a render hit or missed diff --git a/actionview/test/activerecord/polymorphic_routes_test.rb b/actionview/test/activerecord/polymorphic_routes_test.rb index e99c769dc2..b2e0fb08c4 100644 --- a/actionview/test/activerecord/polymorphic_routes_test.rb +++ b/actionview/test/activerecord/polymorphic_routes_test.rb @@ -730,3 +730,49 @@ class PolymorphicPathRoutesTest < PolymorphicRoutesTest assert_equal url.sub(/http:\/\/#{host}/, ""), url_for(args) end end + +class DirectRoutesTest < ActionView::TestCase + class Linkable + attr_reader :id + + def self.name + super.demodulize + end + + def initialize(id) + @id = id + end + + def linkable_type + self.class.name.underscore + end + end + + class Category < Linkable; end + class Collection < Linkable; end + class Product < Linkable; end + + Routes = ActionDispatch::Routing::RouteSet.new + Routes.draw do + resources :categories, :collections, :products + direct(:linkable) { |linkable| [:"#{linkable.linkable_type}", { id: linkable.id }] } + end + + include Routes.url_helpers + + def setup + @category = Category.new("1") + @collection = Collection.new("2") + @product = Product.new("3") + end + + def test_direct_routes + assert_equal "/categories/1", linkable_path(@category) + assert_equal "/collections/2", linkable_path(@collection) + assert_equal "/products/3", linkable_path(@product) + + assert_equal "http://test.host/categories/1", linkable_url(@category) + assert_equal "http://test.host/collections/2", linkable_url(@collection) + assert_equal "http://test.host/products/3", linkable_url(@product) + end +end diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index d561745611..41505fbf83 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -6,10 +6,6 @@ *Yuji Yaginuma* -* Push skipped jobs to `enqueued_jobs` when using `perform_enqueued_jobs` with a `only` filter in tests - - *Alexander Pauly* - * Removed deprecated support to passing the adapter class to `.queue_adapter`. *Rafael Mendonça França* diff --git a/activejob/lib/active_job/queue_adapters/test_adapter.rb b/activejob/lib/active_job/queue_adapters/test_adapter.rb index ec825f12cd..1b633b210e 100644 --- a/activejob/lib/active_job/queue_adapters/test_adapter.rb +++ b/activejob/lib/active_job/queue_adapters/test_adapter.rb @@ -24,27 +24,30 @@ module ActiveJob end def enqueue(job) #:nodoc: + return if filtered?(job) + job_data = job_to_hash(job) enqueue_or_perform(perform_enqueued_jobs, job, job_data) end def enqueue_at(job, timestamp) #:nodoc: + return if filtered?(job) + job_data = job_to_hash(job, at: timestamp) enqueue_or_perform(perform_enqueued_at_jobs, job, job_data) end private - def job_to_hash(job, extras = {}) { job: job.class, args: job.serialize.fetch("arguments"), queue: job.queue_name }.merge!(extras) end def enqueue_or_perform(perform, job, job_data) - if !perform || filtered?(job) - enqueued_jobs << job_data - else + if perform performed_jobs << job_data Base.execute job.serialize + else + enqueued_jobs << job_data end end diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index 2e6357f824..81e75b4374 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -56,17 +56,6 @@ class EnqueuedJobsTest < ActiveJob::TestCase end end - def test_assert_enqueued_jobs_when_performing_with_only_option - assert_nothing_raised do - assert_enqueued_jobs 1, only: HelloJob do - perform_enqueued_jobs only: LoggingJob do - HelloJob.perform_later("sean") - LoggingJob.perform_later("yves") - end - end - end - end - def test_assert_no_enqueued_jobs_with_no_block assert_nothing_raised do assert_no_enqueued_jobs diff --git a/activemodel/lib/active_model/type.rb b/activemodel/lib/active_model/type.rb index b8e6d2376b..095801d8f0 100644 --- a/activemodel/lib/active_model/type.rb +++ b/activemodel/lib/active_model/type.rb @@ -21,16 +21,8 @@ module ActiveModel class << self attr_accessor :registry # :nodoc: - delegate :add_modifier, to: :registry - # Add a new type to the registry, allowing it to be referenced as a - # symbol by ActiveRecord::Attributes::ClassMethods#attribute. If your - # type is only meant to be used with a specific database adapter, you can - # do so by passing +adapter: :postgresql+. If your type has the same - # name as a native type for the current adapter, an exception will be - # raised unless you specify an +:override+ option. +override: true+ will - # cause your type to be used instead of the native type. +override: - # false+ will cause the native type to be used over yours if one exists. + # Add a new type to the registry, allowing it to be get through ActiveModel::Type#lookup def register(type_name, klass = nil, **options, &block) registry.register(type_name, klass, **options, &block) end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 96094a285f..4488524c4e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,24 @@ +* Remove comments from structure.sql when using postgresql adapter to avoid + version-specific parts of the file. + + Fixes #28153. + + *Ari Pollak* + +* Add `:default` option to `belongs_to`. + + Use it to specify that an association should be initialized with a particular + record before validation. For example: + + # Before + belongs_to :account + before_validation -> { self.account ||= Current.account } + + # After + belongs_to :account, default: -> { Current.account } + + *George Claghorn* + * Deprecate `Migrator.schema_migrations_table_name`. *Ryuta Kamizono* @@ -495,11 +516,6 @@ *Prathamesh Sonpatki* -* Optimistic locking: Added ability to update `locking_column` value. - Ignore optimistic locking if trying to update with new `locking_column` value. - - *bogdanvlviv* - * Fixed: Optimistic locking does not work well with `null` in the database. Fixes #26024. @@ -733,7 +749,7 @@ *Johannes Opper* -* Introduce `ActiveRecord::TransactionSerializationError` for catching +* Introduce new Active Record transaction error classes for catching transaction serialization failures or deadlocks. *Erol Fornoles* diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 4606c91ffd..6efa448d49 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1647,6 +1647,9 @@ module ActiveRecord # +:inverse_of+ to avoid an extra query during validation. # NOTE: <tt>required</tt> is set to <tt>true</tt> by default and is deprecated. If # you don't want to have association presence validated, use <tt>optional: true</tt>. + # [:default] + # Provide a callable (i.e. proc or lambda) to specify that the association should + # be initialized with a particular record before validation. # # Option examples: # belongs_to :firm, foreign_key: "client_of" @@ -1660,6 +1663,7 @@ module ActiveRecord # belongs_to :comment, touch: true # belongs_to :company, touch: :employees_last_updated_at # belongs_to :user, optional: true + # belongs_to :account, default: -> { company.account } def belongs_to(name, scope = nil, options = {}) reflection = Builder::BelongsTo.build(self, name, scope, options) Reflection.add_reflection self, name, reflection diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 64b2311911..a2432e389a 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -21,6 +21,10 @@ module ActiveRecord self.target = record end + def default(&block) + writer(instance_exec(&block)) if reader.nil? + end + def reset super @updated = false diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index a1609ab0fb..2b9dd8aae8 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -5,7 +5,7 @@ module ActiveRecord::Associations::Builder # :nodoc: end def self.valid_options(options) - super + [:polymorphic, :touch, :counter_cache, :optional] + super + [:polymorphic, :touch, :counter_cache, :optional, :default] end def self.valid_dependent_options @@ -16,6 +16,7 @@ module ActiveRecord::Associations::Builder # :nodoc: super add_counter_cache_callbacks(model, reflection) if reflection.options[:counter_cache] add_touch_callbacks(model, reflection) if reflection.options[:touch] + add_default_callbacks(model, reflection) if reflection.options[:default] end def self.define_accessors(mixin, reflection) @@ -118,6 +119,12 @@ module ActiveRecord::Associations::Builder # :nodoc: model.after_destroy callback.(:changes_to_save) end + def self.add_default_callbacks(model, reflection) + model.before_validation lambda { |o| + o.association(reflection.name).default(&reflection.options[:default]) + } + end + def self.add_destroy_callbacks(model, reflection) model.after_destroy lambda { |o| o.association(reflection.name).handle_dependency } end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index bc2f359c65..3c44296d8c 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -1154,8 +1154,9 @@ module ActiveRecord end def method_missing(method, *args, &block) - if scope.respond_to?(method) - scope.public_send(method, *args, &block) + if scope.respond_to?(method) && scope.extending_values.any? + extend(*scope.extending_values) + public_send(method, *args, &block) else super end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index e5a24b2aca..6019e05c4c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -155,7 +155,11 @@ module ActiveRecord private def type_casted_binds(binds) - binds.map { |attr| type_cast(attr.value_for_database) } + if binds.first.is_a?(Array) + binds.map { |column, value| type_cast(value, column) } + else + binds.map { |attr| type_cast(attr.value_for_database) } + end end def id_value_for_database(value) 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 1e826ff5ad..1cc10e3b93 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -105,10 +105,12 @@ module ActiveRecord indexes(table_name).any? { |i| checks.all? { |check| check[i] } } end - # Returns an array of Column objects for the table specified by +table_name+. - # See the concrete implementation for details on the expected parameter values. + # Returns an array of +Column+ objects for the table specified by +table_name+. def columns(table_name) - raise NotImplementedError, "#columns is not implemented" + table_name = table_name.to_s + column_definitions(table_name).map do |field| + new_column_from_field(table_name, field) + end end # Checks to see if a column exists in a given table. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index ef1d9f81a9..b9fbcd05c7 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -162,14 +162,6 @@ module ActiveRecord SchemaCreation.new self end - # Returns an array of +Column+ objects for the table specified by +table_name+. - def columns(table_name) # :nodoc: - table_name = table_name.to_s - column_definitions(table_name).map do |field| - new_column_from_field(table_name, field) - end - end - # this method must only be called while holding connection pool's mutex def lease if in_use? diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 0028dc0edb..8f78330d4a 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -559,7 +559,6 @@ module ActiveRecord @marked_for_destruction = false @destroyed_by_association = nil @new_record = true - @txn = nil @_start_transaction_state = {} @transaction_state = nil end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 2659c60f1f..78ce9f8291 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -47,8 +47,6 @@ module ActiveRecord # self.locking_column = :lock_person # end # - # Please note that the optimistic locking will be ignored if you update the - # locking column's value. module Optimistic extend ActiveSupport::Concern @@ -80,13 +78,11 @@ module ActiveRecord def _update_record(attribute_names = self.attribute_names) return super unless locking_enabled? - - lock_col = self.class.locking_column - - return super if attribute_names.include?(lock_col) return 0 if attribute_names.empty? begin + lock_col = self.class.locking_column + previous_lock_value = read_attribute_before_type_cast(lock_col) increment_lock diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 36689f6559..c4a22398f0 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -9,7 +9,7 @@ module ActiveRecord delegate :find_each, :find_in_batches, :in_batches, to: :all delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :left_joins, :left_outer_joins, :or, :where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly, - :having, :create_with, :uniq, :distinct, :references, :none, :unscope, :merge, to: :all + :having, :create_with, :distinct, :references, :none, :unscope, :merge, to: :all delegate :count, :average, :minimum, :maximum, :sum, :calculate, to: :all delegate :pluck, :ids, to: :all diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 0612151584..50378f9d99 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -36,7 +36,7 @@ module ActiveRecord # may vary depending on the klass of a relation, so we create a subclass of Relation # for each different klass, and the delegations are compiled into that subclass only. - delegate :to_xml, :encode_with, :length, :collect, :map, :each, :all?, :include?, :to_ary, :join, + delegate :to_xml, :encode_with, :length, :each, :uniq, :to_ary, :join, :[], :&, :|, :+, :-, :sample, :reverse, :compact, :in_groups, :in_groups_of, :to_sentence, :to_formatted_s, :as_json, :shuffle, :split, :index, to: :records diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 4ee413c805..1178dec706 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1130,7 +1130,12 @@ module ActiveRecord arel_attribute(arg).asc when Hash arg.map { |field, dir| - arel_attribute(field).send(dir.downcase) + case field + when Arel::Nodes::SqlLiteral + field.send(dir.downcase) + else + arel_attribute(field).send(dir.downcase) + end } else arg diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb index 5155ced0e2..f1af90c1e8 100644 --- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb @@ -1,8 +1,11 @@ +require "tempfile" + module ActiveRecord module Tasks # :nodoc: class PostgreSQLDatabaseTasks # :nodoc: DEFAULT_ENCODING = ENV["CHARSET"] || "utf8" ON_ERROR_STOP_1 = "ON_ERROR_STOP=1".freeze + SQL_COMMENT_BEGIN = "--".freeze delegate :connection, :establish_connection, :clear_active_connections!, to: ActiveRecord::Base @@ -65,6 +68,7 @@ module ActiveRecord end args << configuration["database"] run_cmd("pg_dump", args, "dumping") + remove_sql_header_comments(filename) File.open(filename, "a") { |f| f << "SET search_path TO #{connection.schema_search_path};\n\n" } end @@ -110,6 +114,22 @@ module ActiveRecord msg << "Please check the output above for any errors and make sure that `#{cmd}` is installed in your PATH and has proper permissions.\n\n" msg end + + def remove_sql_header_comments(filename) + removing_comments = true + tempfile = Tempfile.open("uncommented_structure.sql") + begin + File.foreach(filename) do |line| + unless removing_comments && (line.start_with?(SQL_COMMENT_BEGIN) || line.blank?) + tempfile << line + removing_comments = false + end + end + ensure + tempfile.close + end + FileUtils.mv(tempfile.path, filename) + end end end end diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 070fca240f..601d575c0e 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -216,6 +216,15 @@ module ActiveRecord assert result.is_a?(ActiveRecord::Result) end + if ActiveRecord::Base.connection.prepared_statements + def test_select_all_with_legacy_binds + post = Post.create!(title: "foo", body: "bar") + expected = @connection.select_all("SELECT * FROM posts WHERE id = #{post.id}") + result = @connection.select_all("SELECT * FROM posts WHERE id = #{Arel::Nodes::BindParam.new.to_sql}", nil, [[nil, post.id]]) + assert_equal expected.to_hash, result.to_hash + end + end + def test_select_methods_passing_a_association_relation author = Author.create!(name: "john") Post.create!(author: author, title: "foo", body: "bar") diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 5875a1871f..5b08ba1358 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -116,6 +116,26 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase ActiveRecord::Base.belongs_to_required_by_default = original_value end + def test_default + david = developers(:david) + jamis = developers(:jamis) + + model = Class.new(ActiveRecord::Base) do + self.table_name = "ships" + def self.name; "Temp"; end + belongs_to :developer, default: -> { david } + end + + ship = model.create! + assert_equal david, ship.developer + + ship = model.create!(developer: jamis) + assert_equal jamis, ship.developer + + ship.update!(developer: nil) + assert_equal david, ship.developer + end + def test_default_scope_on_relations_is_not_cached counter = 0 diff --git a/activerecord/test/cases/associations/extension_test.rb b/activerecord/test/cases/associations/extension_test.rb index 974a3080d4..87d842f21d 100644 --- a/activerecord/test/cases/associations/extension_test.rb +++ b/activerecord/test/cases/associations/extension_test.rb @@ -36,6 +36,11 @@ class AssociationsExtensionsTest < ActiveRecord::TestCase assert_equal comments(:greetings), posts(:welcome).comments.not_again.find_most_recent end + def test_extension_with_dirty_target + comment = posts(:welcome).comments.build(body: "New comment") + assert_equal comment, posts(:welcome).comments.with_content("New comment") + end + def test_marshalling_extensions david = developers(:david) assert_equal projects(:action_controller), david.projects.find_most_recent diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 14f515fa1c..173063e068 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1903,7 +1903,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_calling_many_on_loaded_association_should_not_use_query firm = companies(:first_firm) - firm.clients.collect # force load + firm.clients.load # force load assert_no_queries { assert firm.clients.many? } end @@ -1942,7 +1942,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_calling_none_on_loaded_association_should_not_use_query firm = companies(:first_firm) - firm.clients.collect # force load + firm.clients.load # force load assert_no_queries { assert ! firm.clients.none? } end @@ -1977,7 +1977,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_calling_one_on_loaded_association_should_not_use_query firm = companies(:first_firm) - firm.clients.collect # force load + firm.clients.load # force load assert_no_queries { assert ! firm.clients.one? } end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index c13a962e3e..f9eccfbda1 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -349,13 +349,14 @@ class DirtyTest < ActiveRecord::TestCase def test_partial_update_with_optimistic_locking person = Person.new(first_name: "foo") - old_lock_version = person.lock_version with_partial_writes Person, false do assert_queries(2) { 2.times { person.save! } } Person.where(id: person.id).update_all(first_name: "baz") end + old_lock_version = person.lock_version + with_partial_writes Person, true do assert_queries(0) { 2.times { person.save! } } assert_equal old_lock_version, person.reload.lock_version diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 23095618a4..3a3b8e51f9 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -247,17 +247,6 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal "new title2", t2.title end - def test_lock_without_default_should_update_with_lock_col - t1 = LockWithoutDefault.create(title: "title1", lock_version: 6) - - assert_equal 6, t1.lock_version - - t1.update(lock_version: 0) - t1.reload - - assert_equal 0, t1.lock_version - end - def test_lock_without_default_queries_count t1 = LockWithoutDefault.create(title: "title1") @@ -270,12 +259,6 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal "title2", t1.title assert_equal 1, t1.lock_version - assert_queries(1) { t1.update(title: "title3", lock_version: 6) } - - t1.reload - assert_equal "title3", t1.title - assert_equal 6, t1.lock_version - t2 = LockWithoutDefault.new(title: "title1") assert_queries(1) { t2.save! } @@ -321,17 +304,6 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal "new title2", t2.title end - def test_lock_with_custom_column_without_default_should_update_with_lock_col - t1 = LockWithCustomColumnWithoutDefault.create(title: "title1", custom_lock_version: 6) - - assert_equal 6, t1.custom_lock_version - - t1.update(custom_lock_version: 0) - t1.reload - - assert_equal 0, t1.custom_lock_version - end - def test_lock_with_custom_column_without_default_queries_count t1 = LockWithCustomColumnWithoutDefault.create(title: "title1") @@ -344,12 +316,6 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal "title2", t1.title assert_equal 1, t1.custom_lock_version - assert_queries(1) { t1.update(title: "title3", custom_lock_version: 6) } - - t1.reload - assert_equal "title3", t1.title - assert_equal 6, t1.custom_lock_version - t2 = LockWithCustomColumnWithoutDefault.new(title: "title1") assert_queries(1) { t2.save! } diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 0c94e891eb..73986d2d16 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -112,7 +112,7 @@ class RelationTest < ActiveRecord::TestCase def test_loaded_first topics = Topic.all.order("id ASC") - topics.to_a # force load + topics.load # force load assert_no_queries do assert_equal "The First Topic", topics.first.title @@ -123,7 +123,7 @@ class RelationTest < ActiveRecord::TestCase def test_loaded_first_with_limit topics = Topic.all.order("id ASC") - topics.to_a # force load + topics.load # force load assert_no_queries do assert_equal ["The First Topic", @@ -136,7 +136,7 @@ class RelationTest < ActiveRecord::TestCase def test_first_get_more_than_available topics = Topic.all.order("id ASC") unloaded_first = topics.first(10) - topics.to_a # force load + topics.load # force load assert_no_queries do loaded_first = topics.first(10) @@ -218,17 +218,34 @@ class RelationTest < ActiveRecord::TestCase assert_equal topics(:fifth).title, topics.first.title end - def test_finding_with_reverted_assoc_order + def test_finding_with_arel_assoc_order + topics = Topic.order(Arel.sql("id") => :desc) + assert_equal 5, topics.to_a.size + assert_equal topics(:fifth).title, topics.first.title + end + + def test_finding_with_reversed_assoc_order topics = Topic.order(id: :asc).reverse_order assert_equal 5, topics.to_a.size assert_equal topics(:fifth).title, topics.first.title end + def test_finding_with_reversed_arel_assoc_order + topics = Topic.order(Arel.sql("id") => :asc).reverse_order + assert_equal 5, topics.to_a.size + assert_equal topics(:fifth).title, topics.first.title + end + def test_reverse_order_with_function topics = Topic.order("length(title)").reverse_order assert_equal topics(:second).title, topics.first.title end + def test_reverse_arel_assoc_order_with_function + topics = Topic.order(Arel.sql("length(title)") => :asc).reverse_order + assert_equal topics(:second).title, topics.first.title + end + def test_reverse_order_with_function_other_predicates topics = Topic.order("author_name, length(title), id").reverse_order assert_equal topics(:second).title, topics.first.title @@ -251,6 +268,12 @@ class RelationTest < ActiveRecord::TestCase end end + def test_reverse_arel_assoc_order_with_multiargument_function + assert_nothing_raised do + Topic.order(Arel.sql("REPLACE(title, '', '')") => :asc).reverse_order + end + end + def test_reverse_order_with_nulls_first_or_last assert_raises(ActiveRecord::IrreversibleOrderError) do Topic.order("title NULLS FIRST").reverse_order @@ -1132,7 +1155,7 @@ class RelationTest < ActiveRecord::TestCase assert ! posts.loaded? best_posts = posts.where(comments_count: 0) - best_posts.to_a # force load + best_posts.load # force load assert_no_queries { assert_equal 9, best_posts.size } end @@ -1143,7 +1166,7 @@ class RelationTest < ActiveRecord::TestCase assert ! posts.loaded? best_posts = posts.where(comments_count: 0) - best_posts.to_a # force load + best_posts.load # force load assert_no_queries { assert_equal 9, best_posts.size } end @@ -1153,7 +1176,7 @@ class RelationTest < ActiveRecord::TestCase assert_no_queries { assert_equal 0, posts.size } assert ! posts.loaded? - posts.to_a # force load + posts.load # force load assert_no_queries { assert_equal 0, posts.size } end @@ -1182,7 +1205,7 @@ class RelationTest < ActiveRecord::TestCase assert ! no_posts.loaded? best_posts = posts.where(comments_count: 0) - best_posts.to_a # force load + best_posts.load # force load assert_no_queries { assert_equal false, best_posts.empty? } end diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index 14fb2fbbfa..a6c22ac672 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -10,8 +10,6 @@ require "concurrent/atomic/cyclic_barrier" class DefaultScopingTest < ActiveRecord::TestCase fixtures :developers, :posts, :comments - self.use_transactional_tests = false - def test_default_scope expected = Developer.all.merge!(order: "salary DESC").to_a.collect(&:salary) received = DeveloperOrderedBySalary.all.collect(&:salary) @@ -61,17 +59,6 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal "Jamis", DeveloperCalledJamis.create!.name end - unless in_memory_db? - def test_default_scoping_with_threads - 2.times do - Thread.new { - assert_includes DeveloperOrderedBySalary.all.to_sql, "salary DESC" - DeveloperOrderedBySalary.connection.close - }.join - end - end - end - def test_default_scope_with_inheritance wheres = InheritedPoorDeveloperCalledJamis.all.where_values_hash assert_equal "Jamis", wheres["name"] @@ -435,29 +422,6 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal comment, CommentWithDefaultScopeReferencesAssociation.find_by(id: comment.id) end - unless in_memory_db? - def test_default_scope_is_threadsafe - threads = [] - assert_not_equal 1, ThreadsafeDeveloper.unscoped.count - - barrier_1 = Concurrent::CyclicBarrier.new(2) - barrier_2 = Concurrent::CyclicBarrier.new(2) - - threads << Thread.new do - Thread.current[:default_scope_delay] = -> { barrier_1.wait; barrier_2.wait } - assert_equal 1, ThreadsafeDeveloper.all.to_a.count - ThreadsafeDeveloper.connection.close - end - threads << Thread.new do - Thread.current[:default_scope_delay] = -> { barrier_2.wait } - barrier_1.wait - assert_equal 1, ThreadsafeDeveloper.all.to_a.count - ThreadsafeDeveloper.connection.close - end - threads.each(&:join) - end - end - test "additional conditions are ANDed with the default scope" do scope = DeveloperCalledJamis.where(name: "David") assert_equal 2, scope.where_clause.ast.children.length @@ -506,3 +470,37 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_match gender_pattern, Lion.female.to_sql end end + +class DefaultScopingWithThreadTest < ActiveRecord::TestCase + self.use_transactional_tests = false + + def test_default_scoping_with_threads + 2.times do + Thread.new { + assert_includes DeveloperOrderedBySalary.all.to_sql, "salary DESC" + DeveloperOrderedBySalary.connection.close + }.join + end + end + + def test_default_scope_is_threadsafe + threads = [] + assert_not_equal 1, ThreadsafeDeveloper.unscoped.count + + barrier_1 = Concurrent::CyclicBarrier.new(2) + barrier_2 = Concurrent::CyclicBarrier.new(2) + + threads << Thread.new do + Thread.current[:default_scope_delay] = -> { barrier_1.wait; barrier_2.wait } + assert_equal 1, ThreadsafeDeveloper.all.to_a.count + ThreadsafeDeveloper.connection.close + end + threads << Thread.new do + Thread.current[:default_scope_delay] = -> { barrier_2.wait } + barrier_1.wait + assert_equal 1, ThreadsafeDeveloper.all.to_a.count + ThreadsafeDeveloper.connection.close + end + threads.each(&:join) + end +end unless in_memory_db? diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb index d261fd5321..0c2cffe0d3 100644 --- a/activerecord/test/cases/scoping/named_scoping_test.rb +++ b/activerecord/test/cases/scoping/named_scoping_test.rb @@ -23,8 +23,8 @@ class NamedScopingTest < ActiveRecord::TestCase all_posts = Topic.base assert_queries(1) do - all_posts.collect - all_posts.collect + all_posts.collect { true } + all_posts.collect { true } end end @@ -167,7 +167,7 @@ class NamedScopingTest < ActiveRecord::TestCase def test_first_and_last_should_not_use_query_when_results_are_loaded topics = Topic.base - topics.reload # force load + topics.load # force load assert_no_queries do topics.first topics.last @@ -178,7 +178,7 @@ class NamedScopingTest < ActiveRecord::TestCase topics = Topic.base assert_queries(2) do topics.empty? # use count query - topics.collect # force load + topics.load # force load topics.empty? # use loaded (no query) end end @@ -187,7 +187,7 @@ class NamedScopingTest < ActiveRecord::TestCase topics = Topic.base assert_queries(2) do topics.any? # use count query - topics.collect # force load + topics.load # force load topics.any? # use loaded (no query) end end @@ -203,7 +203,7 @@ class NamedScopingTest < ActiveRecord::TestCase def test_any_should_not_fire_query_if_scope_loaded topics = Topic.base - topics.collect # force load + topics.load # force load assert_no_queries { assert topics.any? } end @@ -217,7 +217,7 @@ class NamedScopingTest < ActiveRecord::TestCase topics = Topic.base assert_queries(2) do topics.many? # use count query - topics.collect # force load + topics.load # force load topics.many? # use loaded (no query) end end @@ -233,7 +233,7 @@ class NamedScopingTest < ActiveRecord::TestCase def test_many_should_not_fire_query_if_scope_loaded topics = Topic.base - topics.collect # force load + topics.load # force load assert_no_queries { assert topics.many? } end @@ -384,7 +384,7 @@ class NamedScopingTest < ActiveRecord::TestCase def test_size_should_use_length_when_results_are_loaded topics = Topic.base - topics.reload # force load + topics.load # force load assert_no_queries do topics.size # use loaded (no query) end diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb index a23100c32a..512388af6b 100644 --- a/activerecord/test/cases/tasks/postgresql_rake_test.rb +++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb @@ -217,17 +217,21 @@ if current_adapter?(:PostgreSQLAdapter) class PostgreSQLStructureDumpTest < ActiveRecord::TestCase def setup - @connection = stub(structure_dump: true) + @connection = stub(schema_search_path: nil, structure_dump: true) @configuration = { "adapter" => "postgresql", "database" => "my-app-db" } - @filename = "awesome-file.sql" + @filename = "/tmp/awesome-file.sql" + FileUtils.touch(@filename) ActiveRecord::Base.stubs(:connection).returns(@connection) ActiveRecord::Base.stubs(:establish_connection).returns(true) Kernel.stubs(:system) - File.stubs(:open) + end + + def teardown + FileUtils.rm_f(@filename) end def test_structure_dump @@ -236,6 +240,15 @@ if current_adapter?(:PostgreSQLAdapter) ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename) end + def test_structure_dump_header_comments_removed + Kernel.stubs(:system).returns(true) + File.write(@filename, "-- header comment\n\n-- more header comment\n statement \n-- lower comment\n") + + ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename) + + assert_equal [" statement \n", "-- lower comment\n"], File.readlines(@filename).first(2) + end + def test_structure_dump_with_extra_flags expected_command = ["pg_dump", "-s", "-x", "-O", "-f", @filename, "--noop", "my-app-db"] diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index e74aedb814..a2028b3eb9 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -59,6 +59,10 @@ class Post < ActiveRecord::Base def the_association proxy_association end + + def with_content(content) + self.detect { |comment| comment.body == content } + end end has_many :comments_with_extend, extend: NamedExtension, class_name: "Comment", foreign_key: "post_id" do diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index de1418bb86..d12cbc34dd 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,26 @@ +* Fixed bug in `DateAndTime::Compatibility#to_time` that caused it to + raise `RuntimeError: can't modify frozen Time` when called on any frozen `Time`. + Properly pass through the frozen `Time` or `ActiveSupport::TimeWithZone` object + when calling `#to_time`. + + *Kevin McPhillips* & *Andrew White* + +* Remove implicit coercion deprecation of durations + + In #28204 we deprecated implicit conversion of durations to a numeric which + represented the number of seconds in the duration because of unwanted side + effects with calculations on durations and dates. This unfortunately had + the side effect of forcing a explicit cast when configuring third-party + libraries like expiration in Redis, e.g: + + redis.expire("foo", 5.minutes) + + To work around this we've removed the deprecation and added a private class + that wraps the numeric and can perform calculation involving durations and + ensure that they remain a duration irrespective of the order of operations. + + *Andrew White* + * Update `titleize` regex to allow apostrophes In 4b685aa the regex in `titleize` was updated to not match apostrophes to diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index 5eee04a34e..e09cee3335 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -156,7 +156,7 @@ module ActiveSupport expires_in = options[:expires_in].to_i if expires_in > 0 && !options[:raw] # Set the memcache expire a few minutes in the future to support race condition ttls on read - expires_in += 300 + expires_in += 5.minutes end rescue_error_with false do @data.send(method, key, value, expires_in, options) diff --git a/activesupport/lib/active_support/core_ext/date_and_time/compatibility.rb b/activesupport/lib/active_support/core_ext/date_and_time/compatibility.rb index db95ae0db5..ab80392460 100644 --- a/activesupport/lib/active_support/core_ext/date_and_time/compatibility.rb +++ b/activesupport/lib/active_support/core_ext/date_and_time/compatibility.rb @@ -10,13 +10,5 @@ module DateAndTime # this behavior, but new apps will have an initializer that sets # this to true, because the new behavior is preferred. mattr_accessor(:preserve_timezone, instance_writer: false) { false } - - def to_time - if preserve_timezone - @_to_time_with_instance_offset ||= getlocal(utc_offset) - else - @_to_time_with_system_offset ||= getlocal - end - end end end diff --git a/activesupport/lib/active_support/core_ext/date_time/compatibility.rb b/activesupport/lib/active_support/core_ext/date_time/compatibility.rb index 30bb7f4a60..eb8b8b2c65 100644 --- a/activesupport/lib/active_support/core_ext/date_time/compatibility.rb +++ b/activesupport/lib/active_support/core_ext/date_time/compatibility.rb @@ -1,5 +1,15 @@ require "active_support/core_ext/date_and_time/compatibility" class DateTime - prepend DateAndTime::Compatibility + include DateAndTime::Compatibility + + remove_possible_method :to_time + + # Either return an instance of `Time` with the same UTC offset + # as +self+ or an instance of `Time` representing the same time + # in the the local system timezone depending on the setting of + # on the setting of +ActiveSupport.to_time_preserves_timezone+. + def to_time + preserve_timezone ? getlocal(utc_offset) : getlocal + end end diff --git a/activesupport/lib/active_support/core_ext/object/with_options.rb b/activesupport/lib/active_support/core_ext/object/with_options.rb index cf39b1d312..3a44e08630 100644 --- a/activesupport/lib/active_support/core_ext/object/with_options.rb +++ b/activesupport/lib/active_support/core_ext/object/with_options.rb @@ -62,6 +62,17 @@ class Object # # Hence the inherited default for `if` key is ignored. # + # NOTE: You cannot call class methods implicitly inside of with_options. + # You can access these methods using the class name instead: + # + # class Phone < ActiveRecord::Base + # enum phone_number_type: [home: 0, office: 1, mobile: 2] + # + # with_options presence: true do + # validates :phone_number_type, inclusion: { in: Phone.phone_number_types.keys } + # end + # end + # def with_options(options, &block) option_merger = ActiveSupport::OptionMerger.new(self, options) block.arity.zero? ? option_merger.instance_eval(&block) : block.call(option_merger) diff --git a/activesupport/lib/active_support/core_ext/time/compatibility.rb b/activesupport/lib/active_support/core_ext/time/compatibility.rb index ca4b9574d5..45e86b77ce 100644 --- a/activesupport/lib/active_support/core_ext/time/compatibility.rb +++ b/activesupport/lib/active_support/core_ext/time/compatibility.rb @@ -1,5 +1,14 @@ require "active_support/core_ext/date_and_time/compatibility" +require "active_support/core_ext/module/remove_method" class Time - prepend DateAndTime::Compatibility + include DateAndTime::Compatibility + + remove_possible_method :to_time + + # Either return +self+ or the time in the local system timezone depending + # on the setting of +ActiveSupport.to_time_preserves_timezone+. + def to_time + preserve_timezone ? self : getlocal + end end diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb index d26bbac511..99080e34a1 100644 --- a/activesupport/lib/active_support/duration.rb +++ b/activesupport/lib/active_support/duration.rb @@ -1,4 +1,5 @@ require "active_support/core_ext/array/conversions" +require "active_support/core_ext/module/delegation" require "active_support/core_ext/object/acts_like" require "active_support/core_ext/string/filters" require "active_support/deprecation" @@ -9,6 +10,66 @@ module ActiveSupport # # 1.month.ago # equivalent to Time.now.advance(months: -1) class Duration + class Scalar < Numeric #:nodoc: + attr_reader :value + delegate :to_i, :to_f, :to_s, to: :value + + def initialize(value) + @value = value + end + + def coerce(other) + [Scalar.new(other), self] + end + + def -@ + Scalar.new(-value) + end + + def <=>(other) + if Scalar === other || Duration === other + value <=> other.value + elsif Numeric === other + value <=> other + else + nil + end + end + + def +(other) + calculate(:+, other) + end + + def -(other) + calculate(:-, other) + end + + def *(other) + calculate(:*, other) + end + + def /(other) + calculate(:/, other) + end + + private + def calculate(op, other) + if Scalar === other + Scalar.new(value.public_send(op, other.value)) + elsif Duration === other + Duration.seconds(value).public_send(op, other) + elsif Numeric === other + Scalar.new(value.public_send(op, other)) + else + raise_type_error(other) + end + end + + def raise_type_error(other) + raise TypeError, "no implicit conversion of #{other.class} into #{self.class}" + end + end + SECONDS_PER_MINUTE = 60 SECONDS_PER_HOUR = 3600 SECONDS_PER_DAY = 86400 @@ -91,12 +152,11 @@ module ActiveSupport end def coerce(other) #:nodoc: - ActiveSupport::Deprecation.warn(<<-MSG.squish) - Implicit coercion of ActiveSupport::Duration to a Numeric - is deprecated and will raise a TypeError in Rails 5.2. - MSG - - [other, value] + if Scalar === other + [other, self] + else + [Scalar.new(other), self] + end end # Compares one Duration with another or a Numeric to this Duration. @@ -132,19 +192,23 @@ module ActiveSupport # Multiplies this Duration by a Numeric and returns a new Duration. def *(other) - if Numeric === other + if Scalar === other || Duration === other + Duration.new(value * other.value, parts.map { |type, number| [type, number * other.value] }) + elsif Numeric === other Duration.new(value * other, parts.map { |type, number| [type, number * other] }) else - value * other + raise_type_error(other) end end # Divides this Duration by a Numeric and returns a new Duration. def /(other) - if Numeric === other + if Scalar === other || Duration === other + Duration.new(value / other.value, parts.map { |type, number| [type, number / other.value] }) + elsif Numeric === other Duration.new(value / other, parts.map { |type, number| [type, number / other] }) else - value / other + raise_type_error(other) end end @@ -274,5 +338,9 @@ module ActiveSupport def method_missing(method, *args, &block) value.send(method, *args, &block) end + + def raise_type_error(other) + raise TypeError, "no implicit conversion of #{other.class} into #{self.class}" + end end end diff --git a/activesupport/lib/active_support/message_encryptor.rb b/activesupport/lib/active_support/message_encryptor.rb index 69109d2005..24053b4fe5 100644 --- a/activesupport/lib/active_support/message_encryptor.rb +++ b/activesupport/lib/active_support/message_encryptor.rb @@ -50,6 +50,11 @@ module ActiveSupport # key by using <tt>ActiveSupport::KeyGenerator</tt> or a similar key # derivation function. # + # First additional parameter is used as the signature key for +MessageVerifier+. + # This allows you to specify keys to encrypt and sign data. + # + # ActiveSupport::MessageEncryptor.new('secret', 'signature_secret') + # # Options: # * <tt>:cipher</tt> - Cipher to use. Can be any cipher returned by # <tt>OpenSSL::Cipher.ciphers</tt>. Default is 'aes-256-cbc'. diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb index e31983cf26..b0dd6b7e8c 100644 --- a/activesupport/lib/active_support/time_with_zone.rb +++ b/activesupport/lib/active_support/time_with_zone.rb @@ -411,6 +411,17 @@ module ActiveSupport @to_datetime ||= utc.to_datetime.new_offset(Rational(utc_offset, 86_400)) end + # Returns an instance of +Time+, either with the same UTC offset + # as +self+ or in the local system timezone depending on the setting + # of +ActiveSupport.to_time_preserves_timezone+. + def to_time + if preserve_timezone + @to_time_with_instance_offset ||= getlocal(utc_offset) + else + @to_time_with_system_offset ||= getlocal + end + end + # So that +self+ <tt>acts_like?(:time)</tt>. def acts_like_time? true @@ -429,7 +440,7 @@ module ActiveSupport def freeze # preload instance variables before freezing - period; utc; time; to_datetime + period; utc; time; to_datetime; to_time super end diff --git a/activesupport/test/core_ext/date_and_time_compatibility_test.rb b/activesupport/test/core_ext/date_and_time_compatibility_test.rb index 4c90460032..6c6205a4d2 100644 --- a/activesupport/test/core_ext/date_and_time_compatibility_test.rb +++ b/activesupport/test/core_ext/date_and_time_compatibility_test.rb @@ -16,11 +16,13 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase def test_time_to_time_preserves_timezone with_preserve_timezone(true) do with_env_tz "US/Eastern" do - time = Time.new(2016, 4, 23, 15, 11, 12, 3600).to_time + source = Time.new(2016, 4, 23, 15, 11, 12, 3600) + time = source.to_time assert_instance_of Time, time assert_equal @utc_time, time.getutc assert_equal @utc_offset, time.utc_offset + assert_equal source.object_id, time.object_id end end end @@ -28,11 +30,43 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase def test_time_to_time_does_not_preserve_time_zone with_preserve_timezone(false) do with_env_tz "US/Eastern" do - time = Time.new(2016, 4, 23, 15, 11, 12, 3600).to_time + source = Time.new(2016, 4, 23, 15, 11, 12, 3600) + time = source.to_time assert_instance_of Time, time assert_equal @utc_time, time.getutc assert_equal @system_offset, time.utc_offset + assert_not_equal source.object_id, time.object_id + end + end + end + + def test_time_to_time_frozen_preserves_timezone + with_preserve_timezone(true) do + with_env_tz "US/Eastern" do + source = Time.new(2016, 4, 23, 15, 11, 12, 3600).freeze + time = source.to_time + + assert_instance_of Time, time + assert_equal @utc_time, time.getutc + assert_equal @utc_offset, time.utc_offset + assert_equal source.object_id, time.object_id + assert_predicate time, :frozen? + end + end + end + + def test_time_to_time_frozen_does_not_preserve_time_zone + with_preserve_timezone(false) do + with_env_tz "US/Eastern" do + source = Time.new(2016, 4, 23, 15, 11, 12, 3600).freeze + time = source.to_time + + assert_instance_of Time, time + assert_equal @utc_time, time.getutc + assert_equal @system_offset, time.utc_offset + assert_not_equal source.object_id, time.object_id + assert_not_predicate time, :frozen? end end end @@ -40,7 +74,8 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase def test_datetime_to_time_preserves_timezone with_preserve_timezone(true) do with_env_tz "US/Eastern" do - time = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1, 24)).to_time + source = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1, 24)) + time = source.to_time assert_instance_of Time, time assert_equal @utc_time, time.getutc @@ -52,7 +87,8 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase def test_datetime_to_time_does_not_preserve_time_zone with_preserve_timezone(false) do with_env_tz "US/Eastern" do - time = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1, 24)).to_time + source = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1, 24)) + time = source.to_time assert_instance_of Time, time assert_equal @utc_time, time.getutc @@ -61,17 +97,47 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase end end + def test_datetime_to_time_frozen_preserves_timezone + with_preserve_timezone(true) do + with_env_tz "US/Eastern" do + source = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1, 24)).freeze + time = source.to_time + + assert_instance_of Time, time + assert_equal @utc_time, time.getutc + assert_equal @utc_offset, time.utc_offset + assert_not_predicate time, :frozen? + end + end + end + + def test_datetime_to_time_frozen_does_not_preserve_time_zone + with_preserve_timezone(false) do + with_env_tz "US/Eastern" do + source = DateTime.new(2016, 4, 23, 15, 11, 12, Rational(1, 24)).freeze + time = source.to_time + + assert_instance_of Time, time + assert_equal @utc_time, time.getutc + assert_equal @system_offset, time.utc_offset + assert_not_predicate time, :frozen? + end + end + end + def test_twz_to_time_preserves_timezone with_preserve_timezone(true) do with_env_tz "US/Eastern" do - time = ActiveSupport::TimeWithZone.new(@utc_time, @zone).to_time + source = ActiveSupport::TimeWithZone.new(@utc_time, @zone) + time = source.to_time assert_instance_of Time, time assert_equal @utc_time, time.getutc assert_instance_of Time, time.getutc assert_equal @utc_offset, time.utc_offset - time = ActiveSupport::TimeWithZone.new(@date_time, @zone).to_time + source = ActiveSupport::TimeWithZone.new(@date_time, @zone) + time = source.to_time assert_instance_of Time, time assert_equal @date_time, time.getutc @@ -84,19 +150,69 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase def test_twz_to_time_does_not_preserve_time_zone with_preserve_timezone(false) do with_env_tz "US/Eastern" do - time = ActiveSupport::TimeWithZone.new(@utc_time, @zone).to_time + source = ActiveSupport::TimeWithZone.new(@utc_time, @zone) + time = source.to_time + + assert_instance_of Time, time + assert_equal @utc_time, time.getutc + assert_instance_of Time, time.getutc + assert_equal @system_offset, time.utc_offset + + source = ActiveSupport::TimeWithZone.new(@date_time, @zone) + time = source.to_time + + assert_instance_of Time, time + assert_equal @date_time, time.getutc + assert_instance_of Time, time.getutc + assert_equal @system_offset, time.utc_offset + end + end + end + + def test_twz_to_time_frozen_preserves_timezone + with_preserve_timezone(true) do + with_env_tz "US/Eastern" do + source = ActiveSupport::TimeWithZone.new(@utc_time, @zone).freeze + time = source.to_time + + assert_instance_of Time, time + assert_equal @utc_time, time.getutc + assert_instance_of Time, time.getutc + assert_equal @utc_offset, time.utc_offset + assert_not_predicate time, :frozen? + + source = ActiveSupport::TimeWithZone.new(@date_time, @zone).freeze + time = source.to_time + + assert_instance_of Time, time + assert_equal @date_time, time.getutc + assert_instance_of Time, time.getutc + assert_equal @utc_offset, time.utc_offset + assert_not_predicate time, :frozen? + end + end + end + + def test_twz_to_time_frozen_does_not_preserve_time_zone + with_preserve_timezone(false) do + with_env_tz "US/Eastern" do + source = ActiveSupport::TimeWithZone.new(@utc_time, @zone).freeze + time = source.to_time assert_instance_of Time, time assert_equal @utc_time, time.getutc assert_instance_of Time, time.getutc assert_equal @system_offset, time.utc_offset + assert_not_predicate time, :frozen? - time = ActiveSupport::TimeWithZone.new(@date_time, @zone).to_time + source = ActiveSupport::TimeWithZone.new(@date_time, @zone).freeze + time = source.to_time assert_instance_of Time, time assert_equal @date_time, time.getutc assert_instance_of Time, time.getutc assert_equal @system_offset, time.utc_offset + assert_not_predicate time, :frozen? end end end @@ -104,7 +220,8 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase def test_string_to_time_preserves_timezone with_preserve_timezone(true) do with_env_tz "US/Eastern" do - time = "2016-04-23T15:11:12+01:00".to_time + source = "2016-04-23T15:11:12+01:00" + time = source.to_time assert_instance_of Time, time assert_equal @utc_time, time.getutc @@ -116,11 +233,40 @@ class DateAndTimeCompatibilityTest < ActiveSupport::TestCase def test_string_to_time_does_not_preserve_time_zone with_preserve_timezone(false) do with_env_tz "US/Eastern" do - time = "2016-04-23T15:11:12+01:00".to_time + source = "2016-04-23T15:11:12+01:00" + time = source.to_time + + assert_instance_of Time, time + assert_equal @utc_time, time.getutc + assert_equal @system_offset, time.utc_offset + end + end + end + + def test_string_to_time_frozen_preserves_timezone + with_preserve_timezone(true) do + with_env_tz "US/Eastern" do + source = "2016-04-23T15:11:12+01:00".freeze + time = source.to_time + + assert_instance_of Time, time + assert_equal @utc_time, time.getutc + assert_equal @utc_offset, time.utc_offset + assert_not_predicate time, :frozen? + end + end + end + + def test_string_to_time_frozen_does_not_preserve_time_zone + with_preserve_timezone(false) do + with_env_tz "US/Eastern" do + source = "2016-04-23T15:11:12+01:00".freeze + time = source.to_time assert_instance_of Time, time assert_equal @utc_time, time.getutc assert_equal @system_offset, time.utc_offset + assert_not_predicate time, :frozen? end end end diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 2b1a715b7a..1648a9b270 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -96,24 +96,20 @@ class DurationTest < ActiveSupport::TestCase assert_instance_of ActiveSupport::Duration, 2.seconds - 1.second assert_equal 1.second, 2.seconds - 1 assert_instance_of ActiveSupport::Duration, 2.seconds - 1 + assert_equal 1.second, 2 - 1.second + assert_instance_of ActiveSupport::Duration, 2.seconds - 1 end def test_multiply assert_equal 7.days, 1.day * 7 assert_instance_of ActiveSupport::Duration, 1.day * 7 - - assert_deprecated do - assert_equal 86400, 1.day * 1.second - end + assert_equal 86400, 1.day * 1.second end def test_divide assert_equal 1.day, 7.days / 7 assert_instance_of ActiveSupport::Duration, 7.days / 7 - - assert_deprecated do - assert_equal 1, 1.day / 1.day - end + assert_equal 1, 1.day / 1.day end def test_date_added_with_multiplied_duration @@ -121,9 +117,7 @@ class DurationTest < ActiveSupport::TestCase end def test_plus_with_time - assert_deprecated do - assert_equal 1 + 1.second, 1.second + 1, "Duration + Numeric should == Numeric + Duration" - end + assert_equal 1 + 1.second, 1.second + 1, "Duration + Numeric should == Numeric + Duration" end def test_time_plus_duration_returns_same_time_datatype @@ -142,13 +136,6 @@ class DurationTest < ActiveSupport::TestCase assert_equal 'expected a time or date, got ""', e.message, "ensure ArgumentError is not being raised by dependencies.rb" end - def test_implicit_coercion_is_deprecated - assert_deprecated { 1 + 1.second } - assert_deprecated { 1 - 1.second } - assert_deprecated { 1 * 1.second } - assert_deprecated { 1 / 1.second } - end - def test_fractional_weeks assert_equal((86400 * 7) * 1.5, 1.5.weeks) assert_equal((86400 * 7) * 1.7, 1.7.weeks) @@ -286,20 +273,125 @@ class DurationTest < ActiveSupport::TestCase def test_comparable assert_equal(-1, (0.seconds <=> 1.second)) assert_equal(-1, (1.second <=> 1.minute)) - - assert_deprecated do - assert_equal(-1, (1 <=> 1.minute)) - end - + assert_equal(-1, (1 <=> 1.minute)) assert_equal(0, (0.seconds <=> 0.seconds)) assert_equal(0, (0.seconds <=> 0.minutes)) assert_equal(0, (1.second <=> 1.second)) assert_equal(1, (1.second <=> 0.second)) assert_equal(1, (1.minute <=> 1.second)) + assert_equal(1, (61 <=> 1.minute)) + end - assert_deprecated do - assert_equal(1, (61 <=> 1.minute)) + def test_implicit_coercion + assert_equal 2.days, 2 * 1.day + assert_instance_of ActiveSupport::Duration, 2 * 1.day + assert_equal Time.utc(2017, 1, 3), Time.utc(2017, 1, 1) + 2 * 1.day + assert_equal Date.civil(2017, 1, 3), Date.civil(2017, 1, 1) + 2 * 1.day + end + + def test_scalar_coerce + scalar = ActiveSupport::Duration::Scalar.new(10) + assert_instance_of ActiveSupport::Duration::Scalar, 10 + scalar + assert_instance_of ActiveSupport::Duration, 10.seconds + scalar + end + + def test_scalar_delegations + scalar = ActiveSupport::Duration::Scalar.new(10) + assert_kind_of Float, scalar.to_f + assert_kind_of Integer, scalar.to_i + assert_kind_of String, scalar.to_s + end + + def test_scalar_unary_minus + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal(-10, -scalar) + assert_instance_of ActiveSupport::Duration::Scalar, -scalar + end + + def test_scalar_compare + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal(1, scalar <=> 5) + assert_equal(0, scalar <=> 10) + assert_equal(-1, scalar <=> 15) + assert_equal(nil, scalar <=> "foo") + end + + def test_scalar_plus + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal 20, 10 + scalar + assert_instance_of ActiveSupport::Duration::Scalar, 10 + scalar + assert_equal 20, scalar + 10 + assert_instance_of ActiveSupport::Duration::Scalar, scalar + 10 + assert_equal 20, 10.seconds + scalar + assert_instance_of ActiveSupport::Duration, 10.seconds + scalar + assert_equal 20, scalar + 10.seconds + assert_instance_of ActiveSupport::Duration, scalar + 10.seconds + + exception = assert_raises(TypeError) do + scalar + "foo" + end + + assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message + end + + def test_scalar_minus + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal 10, 20 - scalar + assert_instance_of ActiveSupport::Duration::Scalar, 20 - scalar + assert_equal 5, scalar - 5 + assert_instance_of ActiveSupport::Duration::Scalar, scalar - 5 + assert_equal 10, 20.seconds - scalar + assert_instance_of ActiveSupport::Duration, 20.seconds - scalar + assert_equal 5, scalar - 5.seconds + assert_instance_of ActiveSupport::Duration, scalar - 5.seconds + + exception = assert_raises(TypeError) do + scalar - "foo" + end + + assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message + end + + def test_scalar_multiply + scalar = ActiveSupport::Duration::Scalar.new(5) + + assert_equal 10, 2 * scalar + assert_instance_of ActiveSupport::Duration::Scalar, 2 * scalar + assert_equal 10, scalar * 2 + assert_instance_of ActiveSupport::Duration::Scalar, scalar * 2 + assert_equal 10, 2.seconds * scalar + assert_instance_of ActiveSupport::Duration, 2.seconds * scalar + assert_equal 10, scalar * 2.seconds + assert_instance_of ActiveSupport::Duration, scalar * 2.seconds + + exception = assert_raises(TypeError) do + scalar * "foo" end + + assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message + end + + def test_scalar_divide + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal 10, 100 / scalar + assert_instance_of ActiveSupport::Duration::Scalar, 100 / scalar + assert_equal 5, scalar / 2 + assert_instance_of ActiveSupport::Duration::Scalar, scalar / 2 + assert_equal 10, 100.seconds / scalar + assert_instance_of ActiveSupport::Duration, 2.seconds * scalar + assert_equal 5, scalar / 2.seconds + assert_instance_of ActiveSupport::Duration, scalar / 2.seconds + + exception = assert_raises(TypeError) do + scalar / "foo" + end + + assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message end def test_twelve_months_equals_one_year diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb index 3cc29ca040..c3afe68378 100644 --- a/activesupport/test/core_ext/time_with_zone_test.rb +++ b/activesupport/test/core_ext/time_with_zone_test.rb @@ -431,11 +431,29 @@ class TimeWithZoneTest < ActiveSupport::TestCase assert_equal time, Time.at(time) end - def test_to_time - with_env_tz "US/Eastern" do - assert_equal Time, @twz.to_time.class - assert_equal Time.local(1999, 12, 31, 19), @twz.to_time - assert_equal Time.local(1999, 12, 31, 19).utc_offset, @twz.to_time.utc_offset + def test_to_time_with_preserve_timezone + with_preserve_timezone(true) do + with_env_tz "US/Eastern" do + time = @twz.to_time + + assert_equal Time, time.class + assert_equal time.object_id, @twz.to_time.object_id + assert_equal Time.local(1999, 12, 31, 19), time + assert_equal Time.local(1999, 12, 31, 19).utc_offset, time.utc_offset + end + end + end + + def test_to_time_without_preserve_timezone + with_preserve_timezone(false) do + with_env_tz "US/Eastern" do + time = @twz.to_time + + assert_equal Time, time.class + assert_equal time.object_id, @twz.to_time.object_id + assert_equal Time.local(1999, 12, 31, 19), time + assert_equal Time.local(1999, 12, 31, 19).utc_offset, time.utc_offset + end end end @@ -518,6 +536,7 @@ class TimeWithZoneTest < ActiveSupport::TestCase @twz.period @twz.time @twz.to_datetime + @twz.to_time end end diff --git a/guides/source/5_1_release_notes.md b/guides/source/5_1_release_notes.md new file mode 100644 index 0000000000..5d4885d55c --- /dev/null +++ b/guides/source/5_1_release_notes.md @@ -0,0 +1,277 @@ +**DO NOT READ THIS FILE ON GITHUB, GUIDES ARE PUBLISHED ON http://guides.rubyonrails.org.** + +Ruby on Rails 5.1 Release Notes +=============================== + +Highlights in Rails 5.1: + +* Yarn Support +* Optional Webpack support +* jQuery no longer a default dependency +* System tests +* Encrypted secrets +* Parameterized mailers +* Direct & resolved routes +* Unification of form_for and form_tag into form_with + +These release notes cover only the major changes. To learn about various bug +fixes and changes, please refer to the change logs or check out the [list of +commits](https://github.com/rails/rails/commits/5-1-stable) in the main Rails +repository on GitHub. + +-------------------------------------------------------------------------------- + +Upgrading to Rails 5.1 +---------------------- + +ToDo + +Major Features +-------------- + +### Yarn Support + +[Pull Request](https://github.com/rails/rails/pull/26836) + +Rails 5.1 will allow managing JavaScript dependencies +from NPM via Yarn. This will make it easy to use libraries like React, VueJS +or any other library from NPM world. The Yarn support is integrated with +the asset pipeline so that all dependencies will work seamlessly with the +Rails 5.1 app. + +### Optional Webpack support + +[Pull Request](https://github.com/rails/rails/pull/27288) + +Rails apps can integrate with [Webpack](https://webpack.js.org/), a JavaScript +asset bundler, more easily using the new [Webpacker](https://github.com/rails/webpacker) +gem. Use the `--webpack` flag when generating new applications to enable Webpack +integration. + +This is fully compatible with the asset pipeline, which you can continue to use for +images, fonts, sounds, and other assets. You can even have some JavaScript code +managed by the asset pipeline, and other code processed via Webpack. All of this is managed +by Yarn, which is enabled by default. + +### jQuery no longer a default dependency + +[Pull Request](https://github.com/rails/rails/pull/27113) + +jQuery was required by default in earlier versions of Rails to provide features +like `data-remote`, `data-confirm` and other parts of Rails' Unobtrusive JavaScript +offerings. It is no longer required, as the UJS has been rewritten to use plain, +vanilla JavaScript. This code now ships inside of Action View as +`rails-ujs`. + +You can still use the jQuery version if needed, but it is no longer required by default. + +### System tests + +[Pull Request](https://github.com/rails/rails/pull/26703) + +Rails 5.1 has baked-in support for writing Capybara tests, in the form of +System tests. You need no longer worry about configuring Capybara and +database cleaning strategies for such tests. Rails 5.1 provides a wrapper +for running tests in Chrome with additional features such as failure +screenshots. + +### Encrypted secrets + +[Pull Request](https://github.com/rails/rails/pull/28038) + +Rails will now allow management of application secrets in a secure way, +building on top of the [sekrets](https://github.com/ahoward/sekrets) gem. + +Run `bin/rails secrets:setup` to setup a new encrypted secrets file. This will +also generate a master key, which must be stored outside of the repository. The +secrets themselves can then be safely checked into the revision control system, +in an encrypted form. + +Secrets will be decrypted in production, using a key stored either in the +`RAILS_MASTER_KEY` environment variable, or in a key file. + +### Parameterized mailers + +[Pull Request](https://github.com/rails/rails/pull/27825) + +Allows specifying common params used for all methods in a mailer class +to share instance variables, headers and other common setup. + +``` ruby +class InvitationsMailer < ApplicationMailer + + before_action { @inviter, @invitee = params[:inviter], params[:invitee] } + before_action { @account = params[:inviter].account } + + def account_invitation + mail subject: "#{@inviter.name} invited you to their Basecamp (#{@account.name})" + end + + def project_invitation + @project = params[:project] + @summarizer = ProjectInvitationSummarizer.new(@project.bucket) + + mail subject: "#{@inviter.name.familiar} added you to a project in Basecamp (#{@account.name})" + end +end + +InvitationsMailer.with(inviter: person_a, invitee: person_b).account_invitation.deliver_later +``` + +### Direct & resolved routes + +[Pull Request](https://github.com/rails/rails/pull/23138) + +Rails 5.1 has added two new methods, `resolve` and `direct`, to the routing +DSL. + +The `resolve` method allows customizing polymorphic mapping of models. + +``` 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`. + +The `direct` method allows creation of custom URL helpers. + +``` ruby +direct(:homepage) { "http://www.rubyonrails.org" } + +>> 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 +``` + +### Unification of form_for and form_tag into form_with + +[Pull Request](https://github.com/rails/rails/pull/26976) + +Before Rails 5.1, there were two interfaces for handling HTML forms: +`form_for` for model instances and `form_tag` for custom URLs. + +Rails 5.1 combines both of these interfaces with `form_with`, and +can generate form tags based on URLs, scopes or models. + +``` erb +# Using just a URL: + +<%= form_with url: posts_path do |form| %> + <%= form.text_field :title %> +<% end %> + +# => +<form action="/posts" method="post" data-remote="true"> + <input type="text" name="title"> +</form> + +# Adding a scope prefixes the input field names: + +<%= form_with scope: :post, url: posts_path do |form| %> + <%= form.text_field :title %> +<% end %> +# => +<form action="/posts" method="post" data-remote="true"> + <input type="text" name="post[title]"> +</form> + +# Using a model infers both the URL and scope: + +<%= form_with model: Post.new do |form| %> + <%= form.text_field :title %> +<% end %> +# => +<form action="/posts" method="post" data-remote="true"> + <input type="text" name="post[title]"> +</form> + +# An existing model makes an update form and fills out field values: + +<%= form_with model: Post.first do |form| %> + <%= form.text_field :title %> +<% end %> +# => +<form action="/posts/1" method="post" data-remote="true"> + <input type="hidden" name="_method" value="patch"> + <input type="text" name="post[title]" value="<the title of the post>"> +</form> +``` + +Railties +-------- + +Please refer to the [Changelog][railties] for detailed changes. + +Action Pack +----------- + +Please refer to the [Changelog][action-pack] for detailed changes. + +Action View +------------- + +Please refer to the [Changelog][action-view] for detailed changes. + +Action Mailer +------------- + +Please refer to the [Changelog][action-mailer] for detailed changes. + +Active Record +------------- + +Please refer to the [Changelog][active-record] for detailed changes. + +Active Model +------------ + +Please refer to the [Changelog][active-model] for detailed changes. + +Active Job +----------- + +Please refer to the [Changelog][active-job] for detailed changes. + +Active Support +-------------- + +Please refer to the [Changelog][active-support] for detailed changes. + +Credits +------- + +See the +[full list of contributors to Rails](http://contributors.rubyonrails.org/) for +the many people who spent many hours making Rails, the stable and robust +framework it is. Kudos to all of them. + +[railties]: https://github.com/rails/rails/blob/5-1-stable/railties/CHANGELOG.md +[action-pack]: https://github.com/rails/rails/blob/5-1-stable/actionpack/CHANGELOG.md +[action-view]: https://github.com/rails/rails/blob/5-1-stable/actionview/CHANGELOG.md +[action-mailer]: https://github.com/rails/rails/blob/5-1-stable/actionmailer/CHANGELOG.md +[action-cable]: https://github.com/rails/rails/blob/5-1-stable/actioncable/CHANGELOG.md +[active-record]: https://github.com/rails/rails/blob/5-1-stable/activerecord/CHANGELOG.md +[active-model]: https://github.com/rails/rails/blob/5-1-stable/activemodel/CHANGELOG.md +[active-support]: https://github.com/rails/rails/blob/5-1-stable/activesupport/CHANGELOG.md +[active-job]: https://github.com/rails/rails/blob/5-1-stable/activejob/CHANGELOG.md diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md index 380fdac658..9673571909 100644 --- a/guides/source/action_mailer_basics.md +++ b/guides/source/action_mailer_basics.md @@ -550,8 +550,9 @@ url helper. <%= user_url(@user, host: 'example.com') %> ``` -NOTE: non-`GET` links require [jQuery UJS](https://github.com/rails/jquery-ujs) -and won't work in mailer templates. They will result in normal `GET` requests. +NOTE: non-`GET` links require [rails-ujs](https://github.com/rails/rails-ujs) or +[jQuery UJS](https://github.com/rails/jquery-ujs), and won't work in mailer templates. +They will result in normal `GET` requests. ### Adding images in Action Mailer Views diff --git a/guides/source/active_job_basics.md b/guides/source/active_job_basics.md index c65d1e6de5..b58ca61848 100644 --- a/guides/source/active_job_basics.md +++ b/guides/source/active_job_basics.md @@ -114,7 +114,7 @@ For enqueuing and executing jobs in production you need to set up a queuing back that is to say you need to decide for a 3rd-party queuing library that Rails should use. Rails itself only provides an in-process queuing system, which only keeps the jobs in RAM. If the process crashes or the machine is reset, then all outstanding jobs are lost with the -default async back-end. This may be fine for smaller apps or non-critical jobs, but most +default async backend. This may be fine for smaller apps or non-critical jobs, but most production apps will need to pick a persistent backend. ### Backends diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 31865ea375..2902c5d677 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -953,9 +953,6 @@ class Client < ApplicationRecord end ``` -NOTE: Please note that the optimistic locking will be ignored if you update the -locking column's value. - ### Pessimistic Locking Pessimistic locking uses a locking mechanism provided by the underlying database. Using `lock` when building a relation obtains an exclusive lock on the selected rows. Relations using `lock` are usually wrapped inside a transaction for preventing deadlock conditions. diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index 32b38cde5e..5313361dfd 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -490,9 +490,6 @@ If you set `:only_integer` to `true`, then it will use the regular expression to validate the attribute's value. Otherwise, it will try to convert the value to a number using `Float`. -WARNING. Note that the regular expression above allows a trailing newline -character. - ```ruby class Player < ApplicationRecord validates :points, numericality: true diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index 68dde4482f..61b7112247 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -335,7 +335,7 @@ an asset has been updated and if so loads it into the page: <%= javascript_include_tag "application", "data-turbolinks-track" => "reload" %> ``` -In regular views you can access images in the `public/assets/images` directory +In regular views you can access images in the `app/assets/images` directory like this: ```erb diff --git a/guides/source/caching_with_rails.md b/guides/source/caching_with_rails.md index fd7626250c..af4ef6a58d 100644 --- a/guides/source/caching_with_rails.md +++ b/guides/source/caching_with_rails.md @@ -396,7 +396,7 @@ config.cache_store = :file_store, "/path/to/cache/directory" ``` With this cache store, multiple server processes on the same host can share a -cache. The cache store is appropriate for low to medium traffic sites that are +cache. This cache store is appropriate for low to medium traffic sites that are served off one or two hosts. Server processes running on different hosts could share a cache by using a shared file system, but that setup is not recommended. @@ -570,6 +570,20 @@ You can also set the strong ETag directly on the response. response.strong_etag = response.body # => "618bbc92e2d35ea1945008b42799b0e7" ``` +Caching in Development +---------------------- + +It's common to want to test the caching strategy of your application +in developement mode. Rails provides the rake task `dev:cache` to +easily toggle caching on/off. + +```bash +$ bin/rails dev:cache +Development mode is now being cached. +$ bin/rails dev:cache +Development mode is no longer being cached. +``` + References ---------- diff --git a/guides/source/debugging_rails_applications.md b/guides/source/debugging_rails_applications.md index ba0cdbf3af..33dee6a868 100644 --- a/guides/source/debugging_rails_applications.md +++ b/guides/source/debugging_rails_applications.md @@ -606,7 +606,6 @@ You can also inspect for an object method this way: @new_record = true @readonly = false @transaction_state = nil -@txn = nil ``` You can also use `display` to start watching variables. This is a good way of diff --git a/guides/source/security.md b/guides/source/security.md index a57c6ea247..a14134f8c1 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -257,13 +257,12 @@ protect_from_forgery with: :exception This will automatically include a security token in all forms and Ajax requests generated by Rails. If the security token doesn't match what was expected, an exception will be thrown. -NOTE: By default, Rails includes jQuery and an [unobtrusive scripting adapter for -jQuery](https://github.com/rails/jquery-ujs), which adds a header called -`X-CSRF-Token` on every non-GET Ajax call made by jQuery with the security token. -Without this header, non-GET Ajax requests won't be accepted by Rails. When using -another library to make Ajax calls, it is necessary to add the security token as -a default header for Ajax calls in your library. To get the token, have a look at -`<meta name='csrf-token' content='THE-TOKEN'>` tag printed by +NOTE: By default, Rails includes an [unobtrusive scripting adapter](https://github.com/rails/rails-ujs), +which adds a header called `X-CSRF-Token` with the security token on every non-GET +Ajax call. Without this header, non-GET Ajax requests won't be accepted by Rails. +When using another library to make Ajax calls, it is necessary to add the security +token as a default header for Ajax calls in your library. To get the token, have +a look at `<meta name='csrf-token' content='THE-TOKEN'>` tag printed by `<%= csrf_meta_tags %>` in your application view. It is common to use persistent cookies to store user information, with `cookies.permanent` for example. In this case, the cookies will not be cleared and the out of the box CSRF protection will not be effective. If you are using a different cookie store than the session for this information, you must handle what to do with it yourself: diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index c1dfcab6f3..e04b3e3581 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -149,7 +149,7 @@ Because of Unobtrusive JavaScript, the Rails "Ajax helpers" are actually in two parts: the JavaScript half and the Ruby half. Unless you have disabled the Asset Pipeline, -[rails.js](https://github.com/rails/jquery-ujs/blob/master/src/rails.js) +[rails-ujs](https://github.com/rails/rails-ujs/blob/master/src/rails-ujs.coffee) provides the JavaScript half, and the regular Ruby view helpers add appropriate tags to your DOM. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index bc5f696675..f4d3529a37 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add `app:update` task to engines. + + *Yuji Yaginuma* + * Avoid running system tests by default with the `bin/rails test` and `bin/rake test` commands since they may be expensive. @@ -126,6 +130,10 @@ *Tsukuru Tanimichi* +* Add `--skip-coffee` option to `rails new` + + *Seunghwan Oh* + * Allow the use of listen's 3.1.x branch *Esteban Santana Santana* diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index b0592151b7..819e801740 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -55,6 +55,34 @@ module Rails @read_encrypted_secrets = false end + def load_defaults(target_version) + case target_version.to_s + when "5.0" + if respond_to?(:action_controller) + action_controller.per_form_csrf_tokens = true + action_controller.forgery_protection_origin_check = true + end + + ActiveSupport.to_time_preserves_timezone = true + + if respond_to?(:active_record) + active_record.belongs_to_required_by_default = true + end + + self.ssl_options = { hsts: { subdomains: true } } + + when "5.1" + load_defaults "5.0" + + if respond_to?(:assets) + assets.unknown_asset_fallback = false + end + + else + raise "Unknown version #{target_version.to_s.inspect}" + end + end + def encoding=(value) @encoding = value silence_warnings do diff --git a/railties/lib/rails/commands/destroy/destroy_command.rb b/railties/lib/rails/commands/destroy/destroy_command.rb index c802910b5d..794673851d 100644 --- a/railties/lib/rails/commands/destroy/destroy_command.rb +++ b/railties/lib/rails/commands/destroy/destroy_command.rb @@ -14,9 +14,9 @@ module Rails return help unless generator require_application_and_environment! - Rails.application.load_generators + load_generators - Rails::Generators.invoke generator, args, behavior: :revoke, destination_root: Rails.root + Rails::Generators.invoke generator, args, behavior: :revoke, destination_root: Rails::Command.root end end end diff --git a/railties/lib/rails/engine/updater.rb b/railties/lib/rails/engine/updater.rb new file mode 100644 index 0000000000..2ecf994a5c --- /dev/null +++ b/railties/lib/rails/engine/updater.rb @@ -0,0 +1,19 @@ +require "rails/generators" +require "rails/generators/rails/plugin/plugin_generator" + +module Rails + class Engine + class Updater + class << self + def generator + @generator ||= Rails::Generators::PluginGenerator.new ["plugin"], + { engine: true }, destination_root: ENGINE_ROOT + end + + def run(action) + generator.send(action) + end + end + end + end +end diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 442258c9d1..38830ba38f 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -371,6 +371,14 @@ module Rails end end + def delete_new_framework_defaults + # Sprockets owns the only new default for 5.1: if it's disabled, + # we don't want the file. + unless options[:update] && !options[:skip_sprockets] + remove_file "config/initializers/new_framework_defaults_5_1.rb" + end + end + def delete_bin_yarn_if_skip_yarn_option remove_file "bin/yarn" if options[:skip_yarn] end diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index b082d70cba..ab4aa04fff 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -34,7 +34,7 @@ group :development, :test do gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] <%- unless options.skip_system_test? || options.api? -%> # Adds support for Capybara system testing and selenium driver - gem 'capybara', '~> 2.7.0' + gem 'capybara', '~> 2.13.0' gem 'selenium-webdriver' <%- end -%> end diff --git a/railties/lib/rails/generators/rails/app/templates/config/application.rb b/railties/lib/rails/generators/rails/app/templates/config/application.rb index c0a0bd0a3e..d5d214052f 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/application.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/application.rb @@ -22,6 +22,9 @@ Bundler.require(*Rails.groups) module <%= app_const_base %> class Application < Rails::Application + # Initialize configuration defaults for originally generated Rails version. + config.load_defaults <%= Rails::VERSION::STRING.to_f %> + # Settings in config/environments/* take precedence over those specified here. # Application configuration should go into files in config/initializers # -- all .rb files in that directory are automatically loaded. diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults.rb.tt deleted file mode 100644 index bd844f0503..0000000000 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults.rb.tt +++ /dev/null @@ -1,37 +0,0 @@ -# Be sure to restart your server when you modify this file. -# -# This file contains migration options to ease your Rails 5.0 upgrade. -# -<%- if options[:update] -%> -# Once upgraded flip defaults one by one to migrate to the new default. -# -<%- end -%> -# Read the Guide for Upgrading Ruby on Rails for more info on each option. -<%- unless options[:api] -%> - -# Enable per-form CSRF tokens. Previous versions had false. -Rails.application.config.action_controller.per_form_csrf_tokens = <%= options[:update] ? false : true %> - -# Enable origin-checking CSRF mitigation. Previous versions had false. -Rails.application.config.action_controller.forgery_protection_origin_check = <%= options[:update] ? false : true %> -<%- end -%> - -# Make Ruby 2.4 preserve the timezone of the receiver when calling `to_time`. -# Previous versions had false. -ActiveSupport.to_time_preserves_timezone = <%= options[:update] ? false : true %> -<%- unless options[:skip_active_record] -%> - -# Require `belongs_to` associations by default. Previous versions had false. -Rails.application.config.active_record.belongs_to_required_by_default = <%= options[:update] ? false : true %> -<%- end -%> -<%- unless options[:update] -%> - -# Configure SSL options to enable HSTS with subdomains. Previous versions had false. -Rails.application.config.ssl_options = { hsts: { subdomains: true } } -<%- end -%> -<%- unless options[:skip_sprockets] -%> - -# Unknown asset fallback will return the path passed in when the given -# asset is not present in the asset pipeline. -Rails.application.config.assets.unknown_asset_fallback = <%= options[:update] ? true : false %> -<%- end -%> diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_1.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_1.rb.tt new file mode 100644 index 0000000000..5f5545c4c7 --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_1.rb.tt @@ -0,0 +1,13 @@ +# Be sure to restart your server when you modify this file. +# +# This file contains migration options to ease your Rails 5.1 upgrade. +# +# Once upgraded flip defaults one by one to migrate to the new default. +# +# Read the Guide for Upgrading Ruby on Rails for more info on each option. +<%- unless options[:skip_sprockets] -%> + +# Unknown asset fallback will return the path passed in when the given +# asset is not present in the asset pipeline. +# Rails.application.config.assets.unknown_asset_fallback = false +<%- end -%> diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index ca48919f9a..118e44d9d0 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -301,7 +301,7 @@ task default: :test end def engine? - full? || mountable? + full? || mountable? || options[:engine] end def full? diff --git a/railties/lib/rails/tasks/engine.rake b/railties/lib/rails/tasks/engine.rake index c92b42f6c1..177b138090 100644 --- a/railties/lib/rails/tasks/engine.rake +++ b/railties/lib/rails/tasks/engine.rake @@ -1,6 +1,17 @@ task "load_app" do namespace :app do load APP_RAKEFILE + + desc "Update some initially generated files" + task update: [ "update:bin" ] + + namespace :update do + require "rails/engine/updater" + # desc "Adds new executables to the engine bin/ directory" + task :bin do + Rails::Engine::Updater.run(:create_bin_files) + end + end end task environment: "app:environment" diff --git a/railties/test/commands/secrets_test.rb b/railties/test/commands/secrets_test.rb index 13fcf6c8a4..00b0343397 100644 --- a/railties/test/commands/secrets_test.rb +++ b/railties/test/commands/secrets_test.rb @@ -17,8 +17,21 @@ class Rails::Command::SecretsCommandTest < ActiveSupport::TestCase assert_match "No $EDITOR to open decrypted secrets in", run_edit_command(editor: "") end + test "edit secrets" do + run_setup_command + + # Run twice to ensure encrypted secrets can be reread after first edit pass. + 2.times do + assert_match(/external_api_key: 1466aac22e6a869134be3d09b9e89232fc2c2289…/, run_edit_command) + end + end + private def run_edit_command(editor: "cat") Dir.chdir(app_path) { `EDITOR="#{editor}" bin/rails secrets:edit` } end + + def run_setup_command + Dir.chdir(app_path) { `bin/rails secrets:setup` } + end end diff --git a/railties/test/generators/api_app_generator_test.rb b/railties/test/generators/api_app_generator_test.rb index bdef1798f8..bb8b29501a 100644 --- a/railties/test/generators/api_app_generator_test.rb +++ b/railties/test/generators/api_app_generator_test.rb @@ -61,15 +61,6 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase end end - def test_generator_skips_per_form_csrf_token_and_origin_check_configs_for_api_apps - run_generator - - assert_file "config/initializers/new_framework_defaults.rb" do |initializer_content| - assert_no_match(/per_form_csrf_tokens/, initializer_content) - assert_no_match(/forgery_protection_origin_check/, initializer_content) - end - end - private def default_files diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 986afb6d2a..ac43e8d22d 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -133,7 +133,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def test_rails_update_generates_correct_session_key + def test_app_update_generates_correct_session_key app_root = File.join(destination_root, "myapp") run_generator [app_root] @@ -156,7 +156,23 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_no_file "config/initializers/cors.rb" end - def test_rails_update_keep_the_cookie_serializer_if_it_is_already_configured + def test_new_application_doesnt_need_defaults + assert_no_file "config/initializers/new_framework_defaults_5_1.rb" + end + + def test_new_application_load_defaults + app_root = File.join(destination_root, "myfirstapp") + run_generator [app_root] + output = nil + + Dir.chdir(app_root) do + output = `./bin/rails r "puts Rails.application.config.assets.unknown_asset_fallback"` + end + + assert_equal "false\n", output + end + + def test_app_update_keep_the_cookie_serializer_if_it_is_already_configured app_root = File.join(destination_root, "myapp") run_generator [app_root] @@ -168,7 +184,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def test_rails_update_set_the_cookie_serializer_to_marshal_if_it_is_not_already_configured + def test_app_update_set_the_cookie_serializer_to_marshal_if_it_is_not_already_configured app_root = File.join(destination_root, "myapp") run_generator [app_root] @@ -183,7 +199,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def test_rails_update_dont_set_file_watcher + def test_app_update_dont_set_file_watcher app_root = File.join(destination_root, "myapp") run_generator [app_root] @@ -197,25 +213,22 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def test_rails_update_does_not_create_new_framework_defaults_by_default + def test_app_update_create_new_framework_defaults app_root = File.join(destination_root, "myapp") run_generator [app_root] - FileUtils.rm("#{app_root}/config/initializers/new_framework_defaults.rb") + assert_no_file "#{app_root}/config/initializers/new_framework_defaults_5_1.rb" stub_rails_application(app_root) do generator = Rails::Generators::AppGenerator.new ["rails"], { update: true }, destination_root: app_root, shell: @shell generator.send(:app_const) quietly { generator.send(:update_config_files) } - assert_file "#{app_root}/config/initializers/new_framework_defaults.rb" do |content| - assert_match(/Rails\.application\.config.active_record\.belongs_to_required_by_default = false/, content) - assert_no_match(/Rails\.application\.config\.ssl_options/, content) - end + assert_file "#{app_root}/config/initializers/new_framework_defaults_5_1.rb" end end - def test_rails_update_does_not_create_rack_cors + def test_app_update_does_not_create_rack_cors app_root = File.join(destination_root, "myapp") run_generator [app_root] @@ -227,7 +240,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def test_rails_update_does_not_remove_rack_cors_if_already_present + def test_app_update_does_not_remove_rack_cors_if_already_present app_root = File.join(destination_root, "myapp") run_generator [app_root] @@ -367,10 +380,6 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "bin/update" do |update_content| assert_no_match(/db:migrate/, update_content) end - - assert_file "config/initializers/new_framework_defaults.rb" do |initializer_content| - assert_no_match(/belongs_to_required_by_default/, initializer_content) - end end def test_generator_if_skip_action_mailer_is_given @@ -415,9 +424,7 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_no_match(/config\.assets\.js_compressor = :uglifier/, content) assert_no_match(/config\.assets\.css_compressor = :sass/, content) end - assert_file "config/initializers/new_framework_defaults.rb" do |content| - assert_no_match(/unknown_asset_fallback/, content) - end + assert_no_file "config/initializers/new_framework_defaults_5_1.rb" end def test_generator_if_skip_yarn_is_given diff --git a/railties/test/generators/encrypted_secrets_generator_test.rb b/railties/test/generators/encrypted_secrets_generator_test.rb index 747abf19ed..21fdcab19f 100644 --- a/railties/test/generators/encrypted_secrets_generator_test.rb +++ b/railties/test/generators/encrypted_secrets_generator_test.rb @@ -12,11 +12,11 @@ class EncryptedSecretsGeneratorTest < Rails::Generators::TestCase def test_generates_key_file_and_encrypted_secrets_file run_generator - assert_file "config/secrets.yml.key", /[\w\d]+/ + assert_file "config/secrets.yml.key", /\w+/ assert File.exist?("config/secrets.yml.enc") - assert_no_match(/production:\n# external_api_key: [\w\d]+/, IO.binread("config/secrets.yml.enc")) - assert_match(/production:\n# external_api_key: [\w\d]+/, Rails::Secrets.read) + assert_no_match(/production:\n# external_api_key: \w+/, IO.binread("config/secrets.yml.enc")) + assert_match(/production:\n# external_api_key: \w+/, Rails::Secrets.read) end def test_appends_to_gitignore diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 8ec096e5c6..afb37b6a99 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -1,6 +1,7 @@ require "generators/generators_test_helper" require "rails/generators/rails/plugin/plugin_generator" require "generators/shared_generator_tests" +require "rails/engine/updater" DEFAULT_PLUGIN_FILES = %w( .gitignore @@ -731,6 +732,21 @@ class PluginGeneratorTest < Rails::Generators::TestCase end end + def test_app_update_generates_bin_file + run_generator [destination_root, "--mountable"] + + Object.const_set("ENGINE_ROOT", destination_root) + FileUtils.rm("#{destination_root}/bin/rails") + + quietly { Rails::Engine::Updater.run(:create_bin_files) } + + assert_file "#{destination_root}/bin/rails" do |content| + assert_match(%r|APP_PATH = File\.expand_path\('\.\./\.\./test/dummy/config/application', __FILE__\)|, content) + end + ensure + Object.send(:remove_const, "ENGINE_ROOT") + end + private def action(*args, &block) diff --git a/railties/test/generators/scaffold_generator_test.rb b/railties/test/generators/scaffold_generator_test.rb index 436fbd5d73..b9c2e791da 100644 --- a/railties/test/generators/scaffold_generator_test.rb +++ b/railties/test/generators/scaffold_generator_test.rb @@ -558,4 +558,59 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase assert_match(/6 runs, 8 assertions, 0 failures, 0 errors/, `bin/rails test 2>&1`) end end + + def test_scaffold_on_invoke_inside_mountable_engine + Dir.chdir(destination_root) { `bundle exec rails plugin new bukkits --mountable` } + engine_path = File.join(destination_root, "bukkits") + + Dir.chdir(engine_path) do + quietly { `bin/rails generate scaffold User name:string age:integer` } + + assert File.exist?("app/models/bukkits/user.rb") + assert File.exist?("test/models/bukkits/user_test.rb") + assert File.exist?("test/fixtures/bukkits/users.yml") + + assert File.exist?("app/controllers/bukkits/users_controller.rb") + assert File.exist?("test/controllers/bukkits/users_controller_test.rb") + + assert File.exist?("app/views/bukkits/users/index.html.erb") + assert File.exist?("app/views/bukkits/users/edit.html.erb") + assert File.exist?("app/views/bukkits/users/show.html.erb") + assert File.exist?("app/views/bukkits/users/new.html.erb") + assert File.exist?("app/views/bukkits/users/_form.html.erb") + + assert File.exist?("app/helpers/bukkits/users_helper.rb") + + assert File.exist?("app/assets/javascripts/bukkits/users.js") + assert File.exist?("app/assets/stylesheets/bukkits/users.css") + end + end + + def test_scaffold_on_revoke_inside_mountable_engine + Dir.chdir(destination_root) { `bundle exec rails plugin new bukkits --mountable` } + engine_path = File.join(destination_root, "bukkits") + + Dir.chdir(engine_path) do + quietly { `bin/rails generate scaffold User name:string age:integer` } + quietly { `bin/rails destroy scaffold User` } + + assert_not File.exist?("app/models/bukkits/user.rb") + assert_not File.exist?("test/models/bukkits/user_test.rb") + assert_not File.exist?("test/fixtures/bukkits/users.yml") + + assert_not File.exist?("app/controllers/bukkits/users_controller.rb") + assert_not File.exist?("test/controllers/bukkits/users_controller_test.rb") + + assert_not File.exist?("app/views/bukkits/users/index.html.erb") + assert_not File.exist?("app/views/bukkits/users/edit.html.erb") + assert_not File.exist?("app/views/bukkits/users/show.html.erb") + assert_not File.exist?("app/views/bukkits/users/new.html.erb") + assert_not File.exist?("app/views/bukkits/users/_form.html.erb") + + assert_not File.exist?("app/helpers/bukkits/users_helper.rb") + + assert_not File.exist?("app/assets/javascripts/bukkits/users.js") + assert_not File.exist?("app/assets/stylesheets/bukkits/users.css") + end + end end diff --git a/railties/test/railties/mounted_engine_test.rb b/railties/test/railties/mounted_engine_test.rb index 5838d0d7e7..6639e55382 100644 --- a/railties/test/railties/mounted_engine_test.rb +++ b/railties/test/railties/mounted_engine_test.rb @@ -141,7 +141,7 @@ module ApplicationTests end def engine_asset_path - render inline: "<%= asset_path 'images/foo.png' %>" + render inline: "<%= asset_path 'images/foo.png', skip_pipeline: true %>" end end end diff --git a/tasks/release.rb b/tasks/release.rb index 8fb151ceb4..b021535245 100644 --- a/tasks/release.rb +++ b/tasks/release.rb @@ -229,7 +229,7 @@ MSG To see the full list of changes, [check out all the commits on GitHub](https://github.com/rails/rails/compare/v#{previous_version}...v#{version}). -## SHA-1 +## SHA-256 If you'd like to verify that your gem is the same as the one I've uploaded, please use these SHA-256 hashes. |