diff options
140 files changed, 1130 insertions, 455 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index 0d2ce7b97f..d12aa0772f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -118,7 +118,7 @@ GEM coffee-script-source (1.10.0) concurrent-ruby (1.0.0) connection_pool (2.2.0) - dalli (2.7.5) + dalli (2.7.6) dante (0.2.0) delayed_job (4.1.1) activesupport (>= 3.0, < 5.0) diff --git a/RELEASING_RAILS.md b/RELEASING_RAILS.md index 83586c22c0..037aa8c119 100644 --- a/RELEASING_RAILS.md +++ b/RELEASING_RAILS.md @@ -36,7 +36,7 @@ Obviously Rails cannot be released when it depends on unreleased code. Contact the authors of those particular gems and work out a release date that suits them. -### Contact the security team (either Koz or tenderlove) +### Contact the security team (either tenderlove or rafaelfranca) Let them know of your plans to release. There may be security issues to be addressed, and that can impact your release date. diff --git a/actioncable/README.md b/actioncable/README.md index 5029f583cb..c85d59a1c8 100644 --- a/actioncable/README.md +++ b/actioncable/README.md @@ -188,7 +188,7 @@ can be reached as remote procedure calls via a subscription's `perform` method. The appearance example was all about exposing server functionality to client-side invocation over the WebSocket connection. But the great thing about WebSockets is that it's a two-way street. So now let's show an example where the server invokes -action on the client. +an action on the client. This is a web notification channel that allows you to trigger client-side web notifications when you broadcast to the right streams: @@ -298,22 +298,24 @@ See the [rails/actioncable-examples](http://github.com/rails/actioncable-example ## Configuration -Action Cable has three required configurations: the Redis connection, allowed request origins, and the cable server url (which can optionally be set on the client side). +Action Cable has three required configurations: a subscription adapter, allowed request origins, and the cable server URL (which can optionally be set on the client side). ### Redis By default, `ActionCable::Server::Base` will look for a configuration file in `Rails.root.join('config/cable.yml')`. -This file must specify a Redis url for each Rails environment. It may use the following format: +This file must specify an adapter and a URL for each Rails environment. It may use the following format: ```yaml production: &production + adapter: redis url: redis://10.10.3.153:6381 development: &development + adapter: redis url: redis://localhost:6379 test: *development ``` -You can also change the location of the Redis config file in a Rails initializer with something like: +You can also change the location of the Action Cable config file in a Rails initializer with something like: ```ruby Rails.application.paths.add "config/cable", with: "somewhere/else/cable.yml" @@ -389,7 +391,7 @@ Also note that your server must provide at least the same number of database con ## Running the cable server ### Standalone -The cable server(s) is separated from your normal application server. It's still a rack application, but it is its own rack +The cable server(s) is separated from your normal application server. It's still a Rack application, but it is its own Rack application. The recommended basic setup is as follows: ```ruby @@ -431,10 +433,7 @@ The WebSocket server doesn't have access to the session, but it has access to th ## Dependencies -Action Cable is currently tied to Redis through its use of the pubsub feature to route -messages back and forth over the WebSocket cable connection. This dependency may well -be alleviated in the future, but for the moment that's what it is. So be sure to have -Redis installed and running. +Action Cable provides a subscription adapter interface to process its pubsub internals. By default, asynchronous, inline, PostgreSQL, evented Redis, and non-evented Redis adapters are included. The default adapter in new Rails applications is the asynchronous (`async`) adapter. To create your own adapter, you can look at `ActionCable::SubscriptionAdapter::Base` for all methods that must be implemented, and any of the adapters included within ActionCable as example implementations. The Ruby side of things is built on top of [websocket-driver](https://github.com/faye/websocket-driver-ruby), [nio4r](https://github.com/celluloid/nio4r), and [concurrent-ruby](https://github.com/ruby-concurrency/concurrent-ruby). diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb index b5f898436a..1acef93025 100644 --- a/actioncable/lib/action_cable/connection/base.rb +++ b/actioncable/lib/action_cable/connection/base.rb @@ -35,7 +35,7 @@ module ActionCable # # First, we declare that this connection can be identified by its current_user. This allows us later to be able to find all connections # established for that current_user (and potentially disconnect them if the user was removed from an account). You can declare as many - # identification indexes as you like. Declaring an identification means that a attr_accessor is automatically set for that key. + # identification indexes as you like. Declaring an identification means that an attr_accessor is automatically set for that key. # # Second, we rely on the fact that the WebSocket connection is established with the cookies from the domain being sent along. This makes # it easy to use signed cookies that were set when logging in via a web interface to authorize the WebSocket connection. @@ -185,12 +185,14 @@ module ActionCable end def respond_to_successful_request + logger.info successful_request_message websocket.rack_response end def respond_to_invalid_request close if websocket.alive? + logger.error invalid_request_message logger.info finished_request_message [ 404, { 'Content-Type' => 'text/plain' }, [ 'Page not found' ] ] end @@ -205,7 +207,7 @@ module ActionCable 'Started %s "%s"%s for %s at %s' % [ request.request_method, request.filtered_path, - websocket.possible? ? ' [WebSocket]' : '', + websocket.possible? ? ' [WebSocket]' : '[non-WebSocket]', request.ip, Time.now.to_s ] end @@ -213,10 +215,22 @@ module ActionCable def finished_request_message 'Finished "%s"%s for %s at %s' % [ request.filtered_path, - websocket.possible? ? ' [WebSocket]' : '', + websocket.possible? ? ' [WebSocket]' : '[non-WebSocket]', request.ip, Time.now.to_s ] end + + def invalid_request_message + 'Failed to upgrade to WebSocket (REQUEST_METHOD: %s, HTTP_CONNECTION: %s, HTTP_UPGRADE: %s)' % [ + env["REQUEST_METHOD"], env["HTTP_CONNECTION"], env["HTTP_UPGRADE"] + ] + end + + def successful_request_message + 'Successfully upgraded to WebSocket (REQUEST_METHOD: %s, HTTP_CONNECTION: %s, HTTP_UPGRADE: %s)' % [ + env["REQUEST_METHOD"], env["HTTP_CONNECTION"], env["HTTP_UPGRADE"] + ] + end end end end diff --git a/actioncable/lib/action_cable/engine.rb b/actioncable/lib/action_cable/engine.rb index f5e233e091..f5f1cb59e0 100644 --- a/actioncable/lib/action_cable/engine.rb +++ b/actioncable/lib/action_cable/engine.rb @@ -31,6 +31,12 @@ module ActionCable self.cable = Rails.application.config_for(config_path).with_indifferent_access end + if 'ApplicationCable::Connection'.safe_constantize + self.connection_class = ApplicationCable::Connection + end + + self.channel_paths = Rails.application.paths['app/channels'].existent + options.each { |k,v| send("#{k}=", v) } end end diff --git a/actioncable/lib/action_cable/server/broadcasting.rb b/actioncable/lib/action_cable/server/broadcasting.rb index 7e8aef45f4..b87232671b 100644 --- a/actioncable/lib/action_cable/server/broadcasting.rb +++ b/actioncable/lib/action_cable/server/broadcasting.rb @@ -23,7 +23,7 @@ module ActionCable broadcaster_for(broadcasting).broadcast(message) end - # Returns a broadcaster for a named <tt>broadcasting</tt> that can be reused. Useful when you have a object that + # Returns a broadcaster for a named <tt>broadcasting</tt> that can be reused. Useful when you have an object that # may need multiple spots to transmit to a specific broadcasting over and over. def broadcaster_for(broadcasting) Broadcaster.new(self, broadcasting) diff --git a/actioncable/lib/action_cable/server/configuration.rb b/actioncable/lib/action_cable/server/configuration.rb index 9a248933c4..58bb8ff65a 100644 --- a/actioncable/lib/action_cable/server/configuration.rb +++ b/actioncable/lib/action_cable/server/configuration.rb @@ -5,27 +5,20 @@ module ActionCable class Configuration attr_accessor :logger, :log_tags attr_accessor :connection_class, :worker_pool_size - attr_accessor :channel_load_paths attr_accessor :disable_request_forgery_protection, :allowed_request_origins attr_accessor :cable, :url + attr_accessor :channel_paths # :nodoc: + def initialize @log_tags = [] - @connection_class = ApplicationCable::Connection - @worker_pool_size = 100 - - @channel_load_paths = [Rails.root.join('app/channels')] + @connection_class = ActionCable::Connection::Base + @worker_pool_size = 100 @disable_request_forgery_protection = false end - def channel_paths - @channel_paths ||= channel_load_paths.flat_map do |path| - Dir["#{path}/**/*_channel.rb"] - end - end - def channel_class_names @channel_class_names ||= channel_paths.collect do |channel_path| Pathname.new(channel_path).basename.to_s.split('.').first.camelize diff --git a/actioncable/test/client_test.rb b/actioncable/test/client_test.rb index 4ade9832e0..d30c381131 100644 --- a/actioncable/test/client_test.rb +++ b/actioncable/test/client_test.rb @@ -12,24 +12,15 @@ class ClientTest < ActionCable::TestCase WAIT_WHEN_NOT_EXPECTING_EVENT = 0.2 def setup - # TODO: ActionCable requires a *lot* of setup at the moment... - ::Object.const_set(:ApplicationCable, Module.new) - ::ApplicationCable.const_set(:Connection, Class.new(ActionCable::Connection::Base)) - - ::Object.const_set(:Rails, Module.new) - ::Rails.singleton_class.send(:define_method, :root) { Pathname.new(__dir__) } - ActionCable.instance_variable_set(:@server, nil) server = ActionCable.server - server.config = ActionCable::Server::Configuration.new - inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } - server.config.logger = ActionCable::Connection::TaggedLoggerProxy.new(inner_logger, tags: []) + server.config.logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } server.config.cable = { adapter: 'async' }.with_indifferent_access # and now the "real" setup for our test: server.config.disable_request_forgery_protection = true - server.config.channel_load_paths = [File.expand_path('client', __dir__)] + server.config.channel_paths = [ File.expand_path('client/echo_channel.rb', __dir__) ] Thread.new { EventMachine.run } unless EventMachine.reactor_running? Thread.pass until EventMachine.reactor_running? @@ -40,15 +31,6 @@ class ClientTest < ActionCable::TestCase def teardown $VERBOSE = @previous_verbose - - begin - ::Object.send(:remove_const, :ApplicationCable) - rescue NameError - end - begin - ::Object.send(:remove_const, :Rails) - rescue NameError - end end def with_puma_server(rack_app = ActionCable.server, port = 3099) diff --git a/actioncable/test/subscription_adapter/common.rb b/actioncable/test/subscription_adapter/common.rb index 361858784e..b31c2aa36c 100644 --- a/actioncable/test/subscription_adapter/common.rb +++ b/actioncable/test/subscription_adapter/common.rb @@ -9,20 +9,7 @@ module CommonSubscriptionAdapterTest WAIT_WHEN_NOT_EXPECTING_EVENT = 0.2 def setup - # TODO: ActionCable requires a *lot* of setup at the moment... - ::Object.const_set(:ApplicationCable, Module.new) - ::ApplicationCable.const_set(:Connection, Class.new(ActionCable::Connection::Base)) - - ::Object.const_set(:Rails, Module.new) - ::Rails.singleton_class.send(:define_method, :root) { Pathname.new(__dir__) } - server = ActionCable::Server::Base.new - server.config = ActionCable::Server::Configuration.new - inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } - server.config.logger = ActionCable::Connection::TaggedLoggerProxy.new(inner_logger, tags: []) - - - # and now the "real" setup for our test: server.config.cable = cable_config.with_indifferent_access adapter_klass = server.config.pubsub_adapter @@ -34,15 +21,6 @@ module CommonSubscriptionAdapterTest def teardown @tx_adapter.shutdown if @tx_adapter && @tx_adapter != @rx_adapter @rx_adapter.shutdown if @rx_adapter - - begin - ::Object.send(:remove_const, :ApplicationCable) - rescue NameError - end - begin - ::Object.send(:remove_const, :Rails) - rescue NameError - end end diff --git a/actioncable/test/worker_test.rb b/actioncable/test/worker_test.rb index 4a699cde27..654f49821e 100644 --- a/actioncable/test/worker_test.rb +++ b/actioncable/test/worker_test.rb @@ -17,7 +17,9 @@ class WorkerTest < ActiveSupport::TestCase end def logger - ActionCable.server.logger + # Impersonating a connection requires a TaggedLoggerProxy'ied logger. + inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } + ActionCable::Connection::TaggedLoggerProxy.new(inner_logger, tags: []) end end diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 22e9bd12a1..1531f2b471 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,9 @@ +* Reset `ActionMailer::Base.deliveries` after every test in + `ActionDispatch::IntegrationTest`. + + *Yves Senn* + + ## Rails 5.0.0.beta2 (February 01, 2016) ## * No changes. diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index bb3cb1be45..4259eb0bee 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -664,7 +664,7 @@ module ActionMailer # # You can also specify overrides if you want by passing a hash instead of a string: # - # mail.attachments['filename.jpg'] = {mime_type: 'application/x-gzip', + # mail.attachments['filename.jpg'] = {mime_type: 'application/gzip', # content: File.read('/path/to/filename.jpg')} # # If you want to use encoding other than Base64 then you will need to pass encoding @@ -672,7 +672,7 @@ module ActionMailer # data: # # file_content = SpecialEncode(File.read('/path/to/filename.jpg')) - # mail.attachments['filename.jpg'] = {mime_type: 'application/x-gzip', + # mail.attachments['filename.jpg'] = {mime_type: 'application/gzip', # encoding: 'SpecialEncoding', # content: file_content } # diff --git a/actionmailer/lib/action_mailer/railtie.rb b/actionmailer/lib/action_mailer/railtie.rb index fa707021c7..22390c4953 100644 --- a/actionmailer/lib/action_mailer/railtie.rb +++ b/actionmailer/lib/action_mailer/railtie.rb @@ -51,6 +51,8 @@ module ActionMailer get '/rails/mailers/*path' => "rails/mailers#preview" end end + + ActionDispatch::IntegrationTest.send :include, ActionMailer::TestCase::ClearTestDeliveries end end diff --git a/actionmailer/lib/action_mailer/test_case.rb b/actionmailer/lib/action_mailer/test_case.rb index 0aa15e31ba..d83719e57d 100644 --- a/actionmailer/lib/action_mailer/test_case.rb +++ b/actionmailer/lib/action_mailer/test_case.rb @@ -11,6 +11,21 @@ module ActionMailer end class TestCase < ActiveSupport::TestCase + module ClearTestDeliveries + extend ActiveSupport::Concern + + included do + teardown :clear_test_deliviers + end + + private + def clear_test_deliviers + if ActionMailer::Base.delivery_method == :test + ActionMailer::Base.deliveries.clear + end + end + end + module Behavior extend ActiveSupport::Concern @@ -66,7 +81,6 @@ module ActionMailer def restore_test_deliveries # :nodoc: restore_delivery_method ActionMailer::Base.perform_deliveries = @old_perform_deliveries - ActionMailer::Base.deliveries.clear end def set_delivery_method(method) # :nodoc: @@ -100,5 +114,6 @@ module ActionMailer end include Behavior + include ClearTestDeliveries end end diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 93e598e493..d473ab427d 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,14 +1,64 @@ +* Add application/gzip as a default mime type. + + *Mehmet Emin İNAÇ* + +* Add request encoding and response parsing to integration tests. + + What previously was: + + ```ruby + require 'test_helper' + + class ApiTest < ActionDispatch::IntegrationTest + test 'creates articles' do + assert_difference -> { Article.count } do + post articles_path(format: :json), + params: { article: { title: 'Ahoy!' } }.to_json, + headers: { 'Content-Type' => 'application/json' } + end + + assert_equal({ 'id' => Article.last.id, 'title' => 'Ahoy!' }, JSON.parse(response.body)) + end + end + ``` + + Can now be written as: + + ```ruby + require 'test_helper' + + class ApiTest < ActionDispatch::IntegrationTest + test 'creates articles' do + assert_difference -> { Article.count } do + post articles_path, params: { article: { title: 'Ahoy!' } }, as: :json + end + + assert_equal({ 'id' => Article.last.id, 'title' => 'Ahoy!' }, response.parsed_body) + end + end + ``` + + Passing `as: :json` to integration test request helpers will set the format, + content type and encode the parameters as JSON. + + Then on the response side, `parsed_body` will parse the body according to the + content type the response has. + + Currently JSON is the only supported MIME type. Add your own with + `ActionDispatch::IntegrationTest.register_encoder`. + + *Kasper Timm Hansen* + * Add image/svg+xml as a default mime type. *DHH* ## Rails 5.0.0.beta2 (February 01, 2016) ## -* Add `-g` and `-c` (short for _grep_ and _controller_ respectively) options - to `bin/rake routes`. These options return the url `name`, `verb` and +* Add `-g` and `-c` options to `bin/rails routes`. These options return the url `name`, `verb` and `path` field that match the pattern or match a specific controller. - Deprecate `CONTROLLER` env variable in `bin/rake routes`. + Deprecate `CONTROLLER` env variable in `bin/rails routes`. See #18902. diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index 450a04779f..a0917b4fdb 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -25,7 +25,7 @@ module ActionController status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name) end message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms" - message << " (#{additions.join(" | ".freeze)})" unless additions.blank? + message << " (#{additions.join(" | ".freeze)})" unless additions.empty? message << "\n\n" if defined?(Rails.env) && Rails.env.development? message diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 3dbf34eb2a..bf74b39ac4 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -19,9 +19,9 @@ module ActionController :controller => self.class.name, :action => self.action_name, :params => request.filtered_parameters, - :format => request.format.try(:ref), + :format => request.format.ref, :method => request.request_method, - :path => (request.fullpath rescue "unknown") + :path => request.fullpath } ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload.dup) diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index d3382ef296..89b4f12ef7 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -109,7 +109,7 @@ module ActionController cattr_accessor :permit_all_parameters, instance_accessor: false cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false - delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, :inspect, + delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, :as_json, to: :@parameters # By default, never raise an UnpermittedParameters exception if these @@ -122,16 +122,6 @@ module ActionController cattr_accessor :always_permitted_parameters self.always_permitted_parameters = %w( controller action ) - def self.const_missing(const_name) - return super unless const_name == :NEVER_UNPERMITTED_PARAMS - ActiveSupport::Deprecation.warn(<<-MSG.squish) - `ActionController::Parameters::NEVER_UNPERMITTED_PARAMS` has been deprecated. - Use `ActionController::Parameters.always_permitted_parameters` instead. - MSG - - always_permitted_parameters - end - # Returns a new instance of <tt>ActionController::Parameters</tt>. # Also, sets the +permitted+ attribute to the default value of # <tt>ActionController::Parameters.permit_all_parameters</tt>. @@ -584,6 +574,10 @@ module ActionController dup end + def inspect + "<#{self.class} #{@parameters}>" + end + def method_missing(method_sym, *args, &block) if @parameters.respond_to?(method_sym) message = <<-DEPRECATE.squish diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index 8356d1a238..66cea88256 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -28,7 +28,8 @@ Mime::Type.register "application/x-www-form-urlencoded", :url_encoded_form # http://www.ietf.org/rfc/rfc4627.txt # http://www.json.org/JSONRequest.html -Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest application/vnd.api+json ) +Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) Mime::Type.register "application/pdf", :pdf, [], %w(pdf) Mime::Type.register "application/zip", :zip, [], %w(zip) +Mime::Type.register "application/gzip", :gzip, %w(application/x-gzip), %w(gz) diff --git a/actionpack/lib/action_dispatch/journey/backwards.rb b/actionpack/lib/action_dispatch/journey/backwards.rb deleted file mode 100644 index 3bd20fdf81..0000000000 --- a/actionpack/lib/action_dispatch/journey/backwards.rb +++ /dev/null @@ -1,5 +0,0 @@ -module Rack # :nodoc: - Mount = ActionDispatch::Journey::Router - Mount::RouteSet = ActionDispatch::Journey::Router - Mount::RegexpWithNamedGroups = ActionDispatch::Journey::Path::Pattern -end diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index f649588520..06cdce1724 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -16,9 +16,6 @@ module ActionDispatch class RoutingError < ::StandardError # :nodoc: end - # :nodoc: - VERSION = '2.0.0' - attr_accessor :routes def initialize(routes) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 3477aa8b29..f2f3150b56 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/hash/keys' require 'active_support/key_generator' require 'active_support/message_verifier' require 'active_support/json' +require 'rack/utils' module ActionDispatch class Request @@ -337,7 +338,7 @@ module ActionDispatch end def to_header - @cookies.map { |k,v| "#{k}=#{v}" }.join ';' + @cookies.map { |k,v| "#{escape(k)}=#{escape(v)}" }.join '; ' end def handle_options(options) #:nodoc: @@ -419,6 +420,10 @@ module ActionDispatch private + def escape(string) + ::Rack::Utils.escape(string) + end + def make_set_cookie_header(header) header = @set_cookies.inject(header) { |m, (k, v)| if write_cookie?(v) diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index b55c937e0c..51a471fb23 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -156,15 +156,20 @@ module ActionDispatch trace = wrapper.framework_trace if trace.empty? ActiveSupport::Deprecation.silence do - message = "\n#{exception.class} (#{exception.message}):\n" - message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) - message << " " << trace.join("\n ") - logger.fatal("#{message}\n\n") + logger.fatal " " + logger.fatal "#{exception.class} (#{exception.message}):" + log_array logger, exception.annoted_source_code if exception.respond_to?(:annoted_source_code) + logger.fatal " " + log_array logger, trace end end + def log_array(logger, array) + array.map { |line| logger.fatal line } + end + def logger(request) - request.logger || stderr_logger + request.logger || ActionView::Base.logger || stderr_logger end def stderr_logger diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 429a98f236..dec9c60ef2 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -23,7 +23,7 @@ module ActionDispatch # goes a step further than signed cookies in that encrypted cookies cannot # be altered or read by users. This is the default starting in Rails 4. # - # If you have both secret_token and secret_key base set, your cookies will + # If you have both secret_token and secret_key_base set, your cookies will # be encrypted, and signed cookies generated by Rails 3 will be # transparently read and encrypted to provide a smooth upgrade path. # diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 6cde5b2900..79f9283f83 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -239,8 +239,7 @@ module ActionDispatch # # rails routes # - # Target specific controllers by prefixing the command with <tt>--controller</tt> option - # - or its <tt>-c</tt> shorthand. + # Target specific controllers by prefixing the command with <tt>-c</tt> option. # module Routing extend ActiveSupport::Autoload diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index b806ee015b..983f1daeb3 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -84,14 +84,15 @@ module ActionDispatch if filter.is_a?(Hash) && filter[:controller] { controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/ } elsif filter - { controller: /#{filter}/, action: /#{filter}/ } + { controller: /#{filter}/, action: /#{filter}/, verb: /#{filter}/, name: /#{filter}/, path: /#{filter}/ } end end def filter_routes(filter) if filter @routes.select do |route| - filter.any? { |default, value| route.defaults[default] =~ value } + route_wrapper = RouteWrapper.new(route) + filter.any? { |default, value| route_wrapper.send(default) =~ value } end else @routes diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 846b5fa1fc..310e98f584 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -289,7 +289,7 @@ module ActionDispatch if last.permitted? args.pop.to_h else - raise ArgumentError, "Generating an URL from non sanitized request parameters is insecure!" + raise ArgumentError, "Generating a URL from non sanitized request parameters is insecure!" end end helper.call self, args, options diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index f91679593e..28be189f93 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -173,7 +173,7 @@ module ActionDispatch route_name) when ActionController::Parameters unless options.permitted? - raise ArgumentError.new("Generating an URL from non sanitized request parameters is insecure!") + raise ArgumentError.new("Generating a URL from non sanitized request parameters is insecure!") end route_name = options.delete :use_route _routes.url_for(options.to_h.symbolize_keys. diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 6f51accee7..f4534b4173 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -321,7 +321,9 @@ module ActionDispatch end # Performs the actual request. - def process(method, path, params: nil, headers: nil, env: nil, xhr: false) + def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: nil) + request_encoder = RequestEncoder.encoder(as) + if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme @@ -330,14 +332,17 @@ module ActionDispatch url_host += ":#{location.port}" if default != location.port host! url_host end - path = location.query ? "#{location.path}?#{location.query}" : location.path + path = request_encoder.append_format_to location.path + path = location.query ? "#{path}?#{location.query}" : path + else + path = request_encoder.append_format_to path end hostname, port = host.split(':') request_env = { :method => method, - :params => params, + :params => request_encoder.encode_params(params), "SERVER_NAME" => hostname, "SERVER_PORT" => port || (https? ? "443" : "80"), @@ -347,7 +352,7 @@ module ActionDispatch "REQUEST_URI" => path, "HTTP_HOST" => host, "REMOTE_ADDR" => remote_addr, - "CONTENT_TYPE" => "application/x-www-form-urlencoded", + "CONTENT_TYPE" => request_encoder.content_type, "HTTP_ACCEPT" => accept } @@ -376,6 +381,7 @@ module ActionDispatch response = _mock_session.last_response @response = ActionDispatch::TestResponse.from_response(response) @response.request = @request + @response.response_parser = RequestEncoder.parser(@response.content_type) @html_document = nil @url_options = nil @@ -387,6 +393,56 @@ module ActionDispatch def build_full_uri(path, env) "#{env['rack.url_scheme']}://#{env['SERVER_NAME']}:#{env['SERVER_PORT']}#{path}" end + + class RequestEncoder # :nodoc: + @encoders = {} + + attr_reader :response_parser + + def initialize(mime_name, param_encoder, response_parser, url_encoded_form = false) + @mime = Mime[mime_name] + + unless @mime + raise ArgumentError, "Can't register a request encoder for " \ + "unregistered MIME Type: #{mime_name}. See `Mime::Type.register`." + end + + @url_encoded_form = url_encoded_form + @path_format = ".#{@mime.symbol}" unless @url_encoded_form + @response_parser = response_parser || -> body { body } + @param_encoder = param_encoder || :"to_#{@mime.symbol}".to_proc + end + + def append_format_to(path) + path << @path_format unless @url_encoded_form + path + end + + def content_type + @mime.to_s + end + + def encode_params(params) + @param_encoder.call(params) + end + + def self.parser(content_type) + mime = Mime::Type.lookup(content_type) + encoder(mime ? mime.ref : nil).response_parser + end + + def self.encoder(name) + @encoders[name] || WWWFormEncoder + end + + def self.register_encoder(mime_name, param_encoder: nil, response_parser: nil) + @encoders[mime_name] = new(mime_name, param_encoder, response_parser) + end + + register_encoder :json, response_parser: -> body { JSON.parse(body) } + + WWWFormEncoder = new(:url_encoded_form, -> params { params }, nil, true) + end end module Runner @@ -643,6 +699,39 @@ module ActionDispatch # end # end # + # You can also test your JSON API easily by setting what the request should + # be encoded as: + # + # require 'test_helper' + # + # class ApiTest < ActionDispatch::IntegrationTest + # test 'creates articles' do + # assert_difference -> { Article.count } do + # post articles_path, params: { article: { title: 'Ahoy!' } }, as: :json + # end + # + # assert_response :success + # assert_equal({ id: Arcticle.last.id, title: 'Ahoy!' }, response.parsed_body) + # end + # end + # + # The `as` option sets the format to JSON, sets the content type to + # 'application/json' and encodes the parameters as JSON. + # + # Calling `parsed_body` on the response parses the response body as what + # the last request was encoded as. If the request wasn't encoded `as` something, + # it's the same as calling `body`. + # + # For any custom MIME Types you've registered, you can even add your own encoders with: + # + # ActionDispatch::IntegrationTest.register_encoder :wibble, + # param_encoder: -> params { params.to_wibble }, + # response_parser: -> body { body } + # + # Where `param_encoder` defines how the params should be encoded and + # `response_parser` defines how the response body should be parsed through + # `parsed_body`. + # # Consult the Rails Testing Guide for more. class IntegrationTest < ActiveSupport::TestCase @@ -671,5 +760,9 @@ module ActionDispatch def document_root_element html_document.root end + + def self.register_encoder(*args) + Integration::Session::RequestEncoder.register_encoder(*args) + end end end diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb index 4b79a90242..9d4b73a43d 100644 --- a/actionpack/lib/action_dispatch/testing/test_response.rb +++ b/actionpack/lib/action_dispatch/testing/test_response.rb @@ -18,5 +18,11 @@ module ActionDispatch # Was there a server-side error? alias_method :error?, :server_error? + + attr_writer :response_parser # :nodoc: + + def parsed_body + @parsed_body ||= @response_parser.call(body) + end end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 74c78dfa8e..7556f984f2 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -390,6 +390,11 @@ class CollectionCacheController < ActionController::Base @customers = [Customer.new('david', 1)] render partial: 'customers/commented_customer', collection: @customers, as: :customer end + + def index_with_callable_cache_key + @customers = [Customer.new('david', 1)] + render @customers, cache: -> customer { 'cached_david' } + end end class AutomaticCollectionCacheTest < ActionController::TestCase @@ -405,6 +410,7 @@ class AutomaticCollectionCacheTest < ActionController::TestCase def test_collection_fetches_cached_views get :index assert_equal 1, @controller.partial_rendered_times + assert_customer_cached 'david/1', 'david, 1' get :index assert_equal 1, @controller.partial_rendered_times @@ -412,8 +418,11 @@ class AutomaticCollectionCacheTest < ActionController::TestCase def test_preserves_order_when_reading_from_cache_plus_rendering get :index, params: { id: 2 } - get :index_ordered + assert_equal 1, @controller.partial_rendered_times + assert_select ':root', 'david, 2' + get :index_ordered + assert_equal 3, @controller.partial_rendered_times assert_select ':root', "david, 1\n david, 2\n david, 3" end @@ -430,6 +439,18 @@ class AutomaticCollectionCacheTest < ActionController::TestCase get :index_with_comment assert_equal 1, @controller.partial_rendered_times end + + def test_caching_with_callable_cache_key + get :index_with_callable_cache_key + assert_customer_cached 'cached_david', 'david, 1' + assert_customer_cached 'david/1', 'david, 1' + end + + private + def assert_customer_cached(key, content) + assert_match content, + ActionView::PartialRenderer.collection_cache.read("views/#{key}/7c228ab609f0baf0b1f2367469210937") + end end class FragmentCacheKeyTestController < CachingController diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index d0a1d1285f..ea50f05f4d 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -1126,3 +1126,69 @@ class IntegrationRequestsWithSessionSetup < ActionDispatch::IntegrationTest assert_equal({"user_name"=>"david"}, cookies.to_hash) end end + +class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest + class FooController < ActionController::Base + def foos_json + render json: params.permit(:foo) + end + + def foos_wibble + render plain: 'ok' + end + end + + def test_encoding_as_json + post_to_foos as: :json do + assert_response :success + assert_match 'foos_json.json', request.path + assert_equal 'application/json', request.content_type + assert_equal({ 'foo' => 'fighters' }, request.request_parameters) + assert_equal({ 'foo' => 'fighters' }, response.parsed_body) + end + end + + def test_encoding_as_without_mime_registration + assert_raise ArgumentError do + ActionDispatch::IntegrationTest.register_encoder :wibble + end + end + + def test_registering_custom_encoder + Mime::Type.register 'text/wibble', :wibble + + ActionDispatch::IntegrationTest.register_encoder(:wibble, + param_encoder: -> params { params }) + + post_to_foos as: :wibble do + assert_response :success + assert_match 'foos_wibble.wibble', request.path + assert_equal 'text/wibble', request.content_type + assert_equal Hash.new, request.request_parameters # Unregistered MIME Type can't be parsed. + assert_equal 'ok', response.parsed_body + end + ensure + Mime::Type.unregister :wibble + end + + def test_parsed_body_without_as_option + with_routing do |routes| + routes.draw { get ':action' => FooController } + + get '/foos_json.json', params: { foo: 'heyo' } + + assert_equal({ 'foo' => 'heyo' }, response.parsed_body) + end + end + + private + def post_to_foos(as:) + with_routing do |routes| + routes.draw { post ':action' => FooController } + + post "/foos_#{as}", params: { foo: 'fighters' }, as: as + + yield + end + end +end diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index bd43ff7697..08b3d81bf0 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -134,4 +134,13 @@ class ParametersAccessorsTest < ActiveSupport::TestCase params1 = ActionController::Parameters.new(hash1) assert(params1 == hash1) end + + test "inspect shows both class name and parameters" do + assert_equal( + '<ActionController::Parameters {"person"=>{"age"=>"32", '\ + '"name"=>{"first"=>"David", "last"=>"Heinemeier Hansson"}, ' \ + '"addresses"=>[{"city"=>"Chicago", "state"=>"Illinois"}]}}>', + @params.inspect + ) + end end diff --git a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb index efaf8a96c3..c5bfb10b53 100644 --- a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb +++ b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb @@ -12,12 +12,6 @@ class AlwaysPermittedParametersTest < ActiveSupport::TestCase ActionController::Parameters.always_permitted_parameters = %w( controller action ) end - test "shows deprecations warning on NEVER_UNPERMITTED_PARAMS" do - assert_deprecated do - ActionController::Parameters::NEVER_UNPERMITTED_PARAMS - end - end - test "returns super on missing constant other than NEVER_UNPERMITTED_PARAMS" do ActionController::Parameters.superclass.stub :const_missing, "super" do assert_equal "super", ActionController::Parameters::NON_EXISTING_CONSTANT diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 0b184eace9..3ea03be74a 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -310,7 +310,7 @@ class RedirectTest < ActionController::TestCase error = assert_raise(ArgumentError) do get :redirect_to_params end - assert_equal "Generating an URL from non sanitized request parameters is insecure!", error.message + assert_equal "Generating a URL from non sanitized request parameters is insecure!", error.message end def test_redirect_to_with_block diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index b9caddcdb7..0c1393548e 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -137,6 +137,10 @@ XML head :created, location: 'created resource' end + def render_cookie + render plain: cookies["foo"] + end + def delete_cookie cookies.delete("foo") render plain: 'ok' @@ -829,6 +833,12 @@ XML assert_equal 'bar', cookies['foo'] end + def test_cookies_should_be_escaped_properly + cookies['foo'] = '+' + get :render_cookie + assert_equal '+', @response.body + end + def test_should_detect_if_cookie_is_deleted cookies['foo'] = 'bar' get :delete_cookie diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index 149e37bf3d..672b272590 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -49,7 +49,7 @@ class MimeTypeTest < ActiveSupport::TestCase test "parse application with trailing star" do accept = "application/*" - expect = [Mime[:html], Mime[:js], Mime[:xml], Mime[:rss], Mime[:atom], Mime[:yaml], Mime[:url_encoded_form], Mime[:json], Mime[:pdf], Mime[:zip]] + expect = [Mime[:html], Mime[:js], Mime[:xml], Mime[:rss], Mime[:atom], Mime[:yaml], Mime[:url_encoded_form], Mime[:json], Mime[:pdf], Mime[:zip], Mime[:gzip]] parsed = Mime::Type.parse(accept) assert_equal expect, parsed end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index a3992ad008..64801bff39 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -37,9 +37,9 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest ) end - test "parses json params for application/vnd.api+json" do + test "does not parse unregistered media types such as application/vnd.api+json" do assert_parses( - {"person" => {"name" => "David"}}, + {}, "{\"person\": {\"name\": \"David\"}}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } ) end @@ -143,13 +143,6 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest ) end - test "parses json params for application/vnd.api+json" do - assert_parses( - {"user" => {"username" => "sikachu"}, "username" => "sikachu"}, - "{\"username\": \"sikachu\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } - ) - end - test "parses json with non-object JSON content" do assert_parses( {"user" => {"_json" => "string content" }, "_json" => "string content" }, diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 256b90784a..bebe78c360 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -68,7 +68,7 @@ *Vasiliy Ermolovich* -* Add a `hidden_field` on the `collection_radio_buttons` to avoid raising a error +* Add a `hidden_field` on the `collection_radio_buttons` to avoid raising an error when the only input on the form is the `collection_radio_buttons`. *Mauro George* diff --git a/actionview/lib/action_view/dependency_tracker.rb b/actionview/lib/action_view/dependency_tracker.rb index 5a4c3ea3fe..7731773040 100644 --- a/actionview/lib/action_view/dependency_tracker.rb +++ b/actionview/lib/action_view/dependency_tracker.rb @@ -7,18 +7,20 @@ module ActionView def self.find_dependencies(name, template, view_paths = nil) tracker = @trackers[template.handler] - return [] unless tracker.present? + return [] unless tracker - if tracker.respond_to?(:supports_view_paths?) && tracker.supports_view_paths? - tracker.call(name, template, view_paths) - else - tracker.call(name, template) - end + tracker.call(name, template, view_paths) end def self.register_tracker(extension, tracker) handler = Template.handler_for_extension(extension) - @trackers[handler] = tracker + if tracker.respond_to?(:supports_view_paths?) + @trackers[handler] = tracker + else + @trackers[handler] = lambda { |name, template, _| + tracker.call(name, template) + } + end end def self.remove_tracker(handler) @@ -151,11 +153,11 @@ module ActionView def resolve_directories(wildcard_dependencies) return [] unless @view_paths - wildcard_dependencies.each_with_object([]) do |query, templates| - @view_paths.find_all_with_query(query).each do |template| - templates << "#{File.dirname(query)}/#{File.basename(template).split('.').first}" + wildcard_dependencies.flat_map { |query, templates| + @view_paths.find_all_with_query(query).map do |template| + "#{File.dirname(query)}/#{File.basename(template).split('.').first}" end - end + }.sort end def explicit_dependencies diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 6f2f9ca53c..657026fa14 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -22,23 +22,23 @@ module ActionView # * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt> # * <tt>dependencies</tt> - An array of dependent views # * <tt>partial</tt> - Specifies whether the template is a partial - def digest(options) - options.assert_valid_keys(:name, :finder, :dependencies, :partial) + def digest(name:, finder:, **options) + options.assert_valid_keys(:dependencies, :partial) - cache_key = ([ options[:name], options[:finder].details_key.hash ].compact + Array.wrap(options[:dependencies])).join('.') + cache_key = ([ name, finder.details_key.hash ].compact + Array.wrap(options[:dependencies])).join('.') # this is a correctly done double-checked locking idiom # (Concurrent::Map's lookups have volatile semantics) @@cache[cache_key] || @@digest_monitor.synchronize do @@cache.fetch(cache_key) do # re-check under lock - compute_and_store_digest(cache_key, options) + compute_and_store_digest(cache_key, name, finder, options) end end end private - def compute_and_store_digest(cache_key, options) # called under @@digest_monitor lock - klass = if options[:partial] || options[:name].include?("/_") + def compute_and_store_digest(cache_key, name, finder, options) # called under @@digest_monitor lock + klass = if options[:partial] || name.include?("/_") # Prevent re-entry or else recursive templates will blow the stack. # There is no need to worry about other threads seeing the +false+ value, # as they will then have to wait for this thread to let go of the @@digest_monitor lock. @@ -48,7 +48,7 @@ module ActionView Digestor end - @@cache[cache_key] = stored_digest = klass.new(options).digest + @@cache[cache_key] = stored_digest = klass.new(name, finder, options).digest ensure # something went wrong or ActionView::Resolver.caching? is false, make sure not to corrupt the @@cache @@cache.delete_pair(cache_key, false) if pre_stored && !stored_digest @@ -57,37 +57,42 @@ module ActionView attr_reader :name, :finder, :options - def initialize(options) - @name, @finder = options.values_at(:name, :finder) - @options = options.except(:name, :finder) + def initialize(name, finder, options = {}) + @name, @finder = name, finder + @options = options end def digest Digest::MD5.hexdigest("#{source}-#{dependency_digest}").tap do |digest| - logger.try :debug, " Cache digest for #{template.inspect}: #{digest}" + logger.debug " Cache digest for #{template.inspect}: #{digest}" end rescue ActionView::MissingTemplate - logger.try :error, " Couldn't find template for digesting: #{name}" + logger.error " Couldn't find template for digesting: #{name}" '' end def dependencies DependencyTracker.find_dependencies(name, template, finder.view_paths) rescue ActionView::MissingTemplate - logger.try :error, " '#{name}' file doesn't exist, so no dependencies" + logger.error " '#{name}' file doesn't exist, so no dependencies" [] end def nested_dependencies dependencies.collect do |dependency| - dependencies = PartialDigestor.new(name: dependency, finder: finder).nested_dependencies + dependencies = PartialDigestor.new(dependency, finder).nested_dependencies dependencies.any? ? { dependency => dependencies } : dependency end end private + class NullLogger + def self.debug(_); end + def self.error(_); end + end + def logger - ActionView::Base.logger + ActionView::Base.logger || NullLogger end def logical_name diff --git a/actionview/lib/action_view/helpers/tag_helper.rb b/actionview/lib/action_view/helpers/tag_helper.rb index 2562504896..42e7358a1d 100644 --- a/actionview/lib/action_view/helpers/tag_helper.rb +++ b/actionview/lib/action_view/helpers/tag_helper.rb @@ -154,6 +154,7 @@ module ActionView options.each_pair do |key, value| if TAG_PREFIXES.include?(key) && value.is_a?(Hash) value.each_pair do |k, v| + next if v.nil? output << sep output << prefix_tag_option(key, k, v, escape) end diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 6a76d80c47..126f289f55 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -22,7 +22,7 @@ module ActionView def self.register_detail(name, &block) self.registered_details << name - initialize = registered_details.map { |n| "@details[:#{n}] = details[:#{n}] || default_#{n}" } + Accessors::DEFAULT_PROCS[name] = block Accessors.send :define_method, :"default_#{name}", &block Accessors.module_eval <<-METHOD, __FILE__, __LINE__ + 1 @@ -34,16 +34,12 @@ module ActionView value = value.present? ? Array(value) : default_#{name} _set_detail(:#{name}, value) if value != @details[:#{name}] end - - remove_possible_method :initialize_details - def initialize_details(details) - #{initialize.join("\n")} - end METHOD end # Holds accessors for the registered details. module Accessors #:nodoc: + DEFAULT_PROCS = {} end register_detail(:locale) do @@ -195,15 +191,23 @@ module ActionView include ViewPaths def initialize(view_paths, details = {}, prefixes = []) - @details, @details_key = {}, nil + @details_key = nil @cache = true @prefixes = prefixes @rendered_format = nil + @details = initialize_details({}, details) self.view_paths = view_paths - initialize_details(details) end + def initialize_details(target, details) + registered_details.each do |k| + target[k] = details[k] || Accessors::DEFAULT_PROCS[k].call + end + target + end + private :initialize_details + # Override formats= to expand ["*/*"] values and automatically # add :html as fallback to :js. def formats=(values) diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index bdbf03191a..a9bd257e76 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -428,6 +428,8 @@ module ActionView layout = find_template(layout, @template_keys) end + collection_cache_eligible = automatic_cache_eligible? + partial_iteration = PartialIteration.new(@collection.size) locals[iteration] = partial_iteration @@ -436,6 +438,11 @@ module ActionView locals[counter] = partial_iteration.index content = template.render(view, locals) + + if collection_cache_eligible + collection_cache_rendered_partial(content, object) + end + content = layout.render(view, locals) { content } if layout partial_iteration.iterate! content diff --git a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb index 1147963882..4860f00243 100644 --- a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb +++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb @@ -15,13 +15,16 @@ module ActionView return yield unless cache_collection? keyed_collection = collection_by_cache_keys - partial_cache = collection_cache.read_multi(*keyed_collection.keys) + cached_partials = collection_cache.read_multi(*keyed_collection.keys) - @collection = keyed_collection.reject { |key, _| partial_cache.key?(key) }.values - rendered_partials = @collection.any? ? yield.dup : [] + @collection = keyed_collection.reject { |key, _| cached_partials.key?(key) }.values + rendered_partials = @collection.empty? ? [] : yield - fetch_or_cache_partial(partial_cache, order_by: keyed_collection.each_key) do - rendered_partials.shift + index = 0 + keyed_collection.map do |cache_key, _| + cached_partials.fetch(cache_key) do + rendered_partials[index].tap { index += 1 } + end end end @@ -30,12 +33,7 @@ module ActionView end def automatic_cache_eligible? - single_template_render? && !callable_cache_key? && - @template.eligible_for_collection_caching?(as: @options[:as]) - end - - def single_template_render? - @template # Template is only set when a collection renders one template. + @template && @template.eligible_for_collection_caching?(as: @options[:as]) end def callable_cache_key? @@ -55,15 +53,10 @@ module ActionView key.frozen? ? key.dup : key # #read_multi & #write may require mutability, Dalli 2.6.0. end - def fetch_or_cache_partial(cached_partials, order_by:) - cache_options = @options[:cache_options] || @locals[:cache_options] || {} - - order_by.map do |key| - cached_partials.fetch(key) do - yield.tap do |rendered_partial| - collection_cache.write(key, rendered_partial, cache_options) - end - end + def collection_cache_rendered_partial(rendered_partial, key_by) + if callable_cache_key? + key = expanded_cache_key(@options[:cache].call(key_by)) + collection_cache.write(key, rendered_partial, @options[:cache_options]) end end end diff --git a/actionview/lib/action_view/tasks/dependencies.rake b/actionview/lib/action_view/tasks/dependencies.rake index f394c319c1..9932ff8b6d 100644 --- a/actionview/lib/action_view/tasks/dependencies.rake +++ b/actionview/lib/action_view/tasks/dependencies.rake @@ -2,13 +2,13 @@ namespace :cache_digests do desc 'Lookup nested dependencies for TEMPLATE (like messages/show or comments/_comment.html)' task :nested_dependencies => :environment do abort 'You must provide TEMPLATE for the task to run' unless ENV['TEMPLATE'].present? - puts JSON.pretty_generate ActionView::Digestor.new(name: CacheDigests.template_name, finder: CacheDigests.finder).nested_dependencies + puts JSON.pretty_generate ActionView::Digestor.new(CacheDigests.template_name, CacheDigests.finder).nested_dependencies end desc 'Lookup first-level dependencies for TEMPLATE (like messages/show or comments/_comment.html)' task :dependencies => :environment do abort 'You must provide TEMPLATE for the task to run' unless ENV['TEMPLATE'].present? - puts JSON.pretty_generate ActionView::Digestor.new(name: CacheDigests.template_name, finder: CacheDigests.finder).dependencies + puts JSON.pretty_generate ActionView::Digestor.new(CacheDigests.template_name, CacheDigests.finder).dependencies end class CacheDigests diff --git a/actionview/lib/action_view/template/error.rb b/actionview/lib/action_view/template/error.rb index b03b197cb5..3f38c3d2b9 100644 --- a/actionview/lib/action_view/template/error.rb +++ b/actionview/lib/action_view/template/error.rb @@ -59,6 +59,9 @@ module ActionView class Error < ActionViewError #:nodoc: SOURCE_CODE_RADIUS = 3 + # Override to prevent #cause resetting during re-raise. + attr_reader :cause + def initialize(template, original_exception = nil) if original_exception ActiveSupport::Deprecation.warn("Passing #original_exception is deprecated and has no effect. " \ @@ -67,6 +70,7 @@ module ActionView super($!.message) set_backtrace($!.backtrace) + @cause = $! @template, @sub_templates = template, nil end @@ -131,13 +135,13 @@ module ActionView end def formatted_code_for(source_code, line_counter, indent, output) - start_value = (output == :html) ? {} : "" + start_value = (output == :html) ? {} : [] source_code.inject(start_value) do |result, line| line_counter += 1 if output == :html result.update(line_counter.to_s => "%#{indent}s %s\n" % ["", line]) else - result << "%#{indent}s: %s\n" % [line_counter, line] + result << "%#{indent}s: %s" % [line_counter, line] end end end diff --git a/actionview/test/fixtures/digestor/messages/peek.html.erb b/actionview/test/fixtures/digestor/messages/peek.html.erb new file mode 100644 index 0000000000..84885ab0bc --- /dev/null +++ b/actionview/test/fixtures/digestor/messages/peek.html.erb @@ -0,0 +1,2 @@ +<%# Template Dependency: messages/message %> +<%= render "comments/comments" %> diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index dde757b5a2..e50caac6d8 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -13,34 +13,11 @@ class FixtureTemplate end end -class FixtureFinder +class FixtureFinder < ActionView::LookupContext FIXTURES_DIR = "#{File.dirname(__FILE__)}/../fixtures/digestor" - attr_reader :details, :view_paths - attr_accessor :formats - attr_accessor :variants - - def initialize - @details = {} - @view_paths = ActionView::PathSet.new(['digestor']) - @formats = [] - @variants = [] - end - - def details_key - details.hash - end - - def find(name, prefixes = [], partial = false, keys = [], options = {}) - partial_name = partial ? name.gsub(%r|/([^/]+)$|, '/_\1') : name - format = @formats.first.to_s - format += "+#{@variants.first}" if @variants.any? - - FixtureTemplate.new("digestor/#{partial_name}.#{format}.erb") - end - - def disable_cache(&block) - yield + def initialize(details = {}) + super(ActionView::PathSet.new(['digestor']), details, []) end end @@ -153,6 +130,16 @@ class TemplateDigestorTest < ActionView::TestCase end end + def test_getting_of_singly_nested_dependencies + singly_nested_dependencies = ["messages/header", "messages/form", "messages/message", "events/event", "comments/comment"] + assert_equal singly_nested_dependencies, nested_dependencies('messages/edit') + end + + def test_getting_of_doubly_nested_dependencies + doubly_nested = [{"comments/comments"=>["comments/comment"]}, "messages/message"] + assert_equal doubly_nested, nested_dependencies('messages/peek') + end + def test_nested_template_directory assert_digest_difference("messages/show") do change_template("messages/actions/_move") @@ -206,13 +193,14 @@ class TemplateDigestorTest < ActionView::TestCase def test_details_are_included_in_cache_key # Cache the template digest. + @finder = FixtureFinder.new({:formats => [:html]}) old_digest = digest("events/_event") # Change the template; the cached digest remains unchanged. change_template("events/_event") # The details are changed, so a new cache key is generated. - finder.details[:foo] = "bar" + @finder = FixtureFinder.new # The cache is busted. assert_not_equal old_digest, digest("events/_event") @@ -317,25 +305,24 @@ class TemplateDigestorTest < ActionView::TestCase yield - assert previous_digest != digest(template_name, options), "digest didn't change" + assert_not_equal previous_digest, digest(template_name, options), "digest didn't change" ActionView::Digestor.cache.clear end def digest(template_name, options = {}) options = options.dup - finder.formats = [:html] finder.variants = options.delete(:variants) || [] ActionView::Digestor.digest({ name: template_name, finder: finder }.merge(options)) end def dependencies(template_name) - ActionView::Digestor.new({ name: template_name, finder: finder }).dependencies + ActionView::Digestor.new(template_name, finder).dependencies end def nested_dependencies(template_name) - ActionView::Digestor.new({ name: template_name, finder: finder }).nested_dependencies + ActionView::Digestor.new(template_name, finder).nested_dependencies end def finder diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index 333e0cca11..bf811abdd0 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -226,13 +226,13 @@ module RenderTestCases assert_match %r!method.*doesnt_exist!, e.message assert_equal "", e.sub_template_message assert_equal "1", e.line_number - assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code.strip + assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code[0].strip assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name end def test_render_error_indentation e = assert_raises(ActionView::Template::Error) { @view.render(:partial => "test/raise_indentation") } - error_lines = e.annoted_source_code.split("\n") + error_lines = e.annoted_source_code assert_match %r!error\shere!, e.message assert_equal "11", e.line_number assert_equal " 9: <p>Ninth paragraph</p>", error_lines.second @@ -252,7 +252,7 @@ module RenderTestCases assert_match %r!method.*doesnt_exist!, e.message assert_equal "", e.sub_template_message assert_equal "1", e.line_number - assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code.strip + assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code[0].strip assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name end diff --git a/actionview/test/template/tag_helper_test.rb b/actionview/test/template/tag_helper_test.rb index 6f7a78ccef..f3956a31f6 100644 --- a/actionview/test/template/tag_helper_test.rb +++ b/actionview/test/template/tag_helper_test.rb @@ -173,4 +173,10 @@ class TagHelperTest < ActionView::TestCase tag('a', { aria => { a_float: 3.14, a_big_decimal: BigDecimal.new("-123.456"), a_number: 1, string: 'hello', symbol: :foo, array: [1, 2, 3], hash: { key: 'value'}, string_with_quotes: 'double"quote"party"' } }) } end + + def test_link_to_data_nil_equal + div_type1 = content_tag(:div, 'test', { 'data-tooltip' => nil }) + div_type2 = content_tag(:div, 'test', { data: {tooltip: nil} }) + assert_dom_equal div_type1, div_type2 + end end diff --git a/activejob/lib/active_job/async_job.rb b/activejob/lib/active_job/async_job.rb index ed7a6e8d9b..417ace21d2 100644 --- a/activejob/lib/active_job/async_job.rb +++ b/activejob/lib/active_job/async_job.rb @@ -6,7 +6,7 @@ require 'concurrent/utility/processor_counter' module ActiveJob # == Active Job Async Job # - # When enqueueing jobs with Async Job each job will be executed asynchronously + # When enqueuing jobs with Async Job each job will be executed asynchronously # on a +concurrent-ruby+ thread pool. All job data is retained in memory. # Because job data is not saved to a persistent datastore there is no # additional infrastructure needed and jobs process quickly. The lack of diff --git a/activejob/lib/active_job/queue_adapters/async_adapter.rb b/activejob/lib/active_job/queue_adapters/async_adapter.rb index 3fc27f56e7..3d3c749883 100644 --- a/activejob/lib/active_job/queue_adapters/async_adapter.rb +++ b/activejob/lib/active_job/queue_adapters/async_adapter.rb @@ -4,7 +4,7 @@ module ActiveJob module QueueAdapters # == Active Job Async adapter # - # When enqueueing jobs with the Async adapter the job will be executed + # When enqueuing jobs with the Async adapter the job will be executed # asynchronously using {AsyncJob}[http://api.rubyonrails.org/classes/ActiveJob/AsyncJob.html]. # # To use +AsyncJob+ set the queue_adapter config to +:async+. diff --git a/activejob/lib/active_job/queue_adapters/inline_adapter.rb b/activejob/lib/active_job/queue_adapters/inline_adapter.rb index 8ad5f4de07..0496f8449e 100644 --- a/activejob/lib/active_job/queue_adapters/inline_adapter.rb +++ b/activejob/lib/active_job/queue_adapters/inline_adapter.rb @@ -2,7 +2,7 @@ module ActiveJob module QueueAdapters # == Active Job Inline adapter # - # When enqueueing jobs with the Inline adapter the job will be executed + # When enqueuing jobs with the Inline adapter the job will be executed # immediately. # # To use the Inline set the queue_adapter config to +:inline+. diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a4cf6023e1..70549243a8 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,46 @@ +* Fix a bug where using `t.foreign_key` twice with the same `to_table` within + the same table definition would only create one foreign key. + + *George Millo* + +* Fix a regression on has many association, where calling a child from parent in child's callback + results in same child records getting added repeatedly to target. + + Fixes #13387. + + *Bogdan Gusiev*, *Jon Hinson* + +* Rework `ActiveRecord::Relation#last` + + 1. Never perform additional SQL on loaded relation + 2. Use SQL reverse order instead of loading relation if relation doesn't have limit + 3. Deprecated relation loading when SQL order can not be automatically reversed + + Topic.order("title").load.last(3) + # before: SELECT ... + # after: No SQL + + Topic.order("title").last + # before: SELECT * FROM `topics` + # after: SELECT * FROM `topics` ORDER BY `topics`.`title` DESC LIMIT 1 + + Topic.order("coalesce(author, title)").last + # before: SELECT * FROM `topics` + # after: Deprecation Warning for irreversible order + + *Bogdan Gusiev* + + +* Allow `joins` to be unscoped. + + Closes #13775. + + *Takashi Kokubun* + +* Add ActiveRecord `#second_to_last` and `#third_to_last` methods. + + *Brian Christian* + * Added `numeric` helper into migrations. Example: @@ -612,7 +655,7 @@ *Ben Murphy*, *Matthew Draper* -* `bin/rake db:migrate` uses +* `bin/rails db:migrate` uses `ActiveRecord::Tasks::DatabaseTasks.migrations_paths` instead of `Migrator.migrations_paths`. diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 9f2c7292ea..2dca6b612e 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -136,6 +136,14 @@ module ActiveRecord first_nth_or_last(:forty_two, *args) end + def third_to_last(*args) + first_nth_or_last(:third_to_last, *args) + end + + def second_to_last(*args) + first_nth_or_last(:second_to_last, *args) + end + def last(*args) first_nth_or_last(:last, *args) end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index fe693cfbb6..2a9627a474 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -197,6 +197,16 @@ module ActiveRecord @association.forty_two(*args) end + # Same as #first except returns only the third-to-last record. + def third_to_last(*args) + @association.third_to_last(*args) + end + + # Same as #first except returns only the second-to-last record. + def second_to_last(*args) + @association.second_to_last(*args) + end + # Returns the last record, or the last +n+ records, from the collection. # If the collection is empty, the first form returns +nil+, and the second # form returns an empty array. diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 708b3af5bd..c5fbe0d1d1 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -54,12 +54,18 @@ module ActiveRecord end scope_chain_index += 1 - relation = ActiveRecord::Relation.create( - klass, - table, - predicate_builder, - ) - scope_chain_items.concat [klass.send(:build_default_scope, relation)].compact + klass_scope = + if klass.current_scope + klass.current_scope.clone + else + relation = ActiveRecord::Relation.create( + klass, + table, + predicate_builder, + ) + klass.send(:build_default_scope, relation) + end + scope_chain_items.concat [klass_scope].compact rel = scope_chain_items.inject(scope_chain_items.shift) do |left, right| left.merge right diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 423a93964e..e902eb7531 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -34,30 +34,6 @@ module ActiveRecord BLACKLISTED_CLASS_METHODS = %w(private public protected allocate new name parent superclass) - class AttributeMethodCache - def initialize - @module = Module.new - @method_cache = Concurrent::Map.new - end - - def [](name) - @method_cache.compute_if_absent(name) do - safe_name = name.unpack('h*'.freeze).first - temp_method = "__temp__#{safe_name}" - ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name - @module.module_eval method_body(temp_method, safe_name), __FILE__, __LINE__ - @module.instance_method temp_method - end - end - - private - - # Override this method in the subclasses for method body. - def method_body(method_name, const_name) - raise NotImplementedError, "Subclasses must implement a method_body(method_name, const_name) method." - end - end - class GeneratedAttributeMethods < Module; end # :nodoc: module ClassMethods diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 5197e21fa4..ab2ecaa7c5 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -1,8 +1,11 @@ module ActiveRecord module AttributeMethods module Read - ReaderMethodCache = Class.new(AttributeMethodCache) { - private + extend ActiveSupport::Concern + + module ClassMethods + protected + # We want to generate the methods via module_eval rather than # define_method, because define_method is slower on dispatch. # Evaluating many similar methods may use more memory as the instruction @@ -21,21 +24,6 @@ module ActiveRecord # to allocate an object on each call to the attribute method. # Making it frozen means that it doesn't get duped when used to # key the @attributes in read_attribute. - def method_body(method_name, const_name) - <<-EOMETHOD - def #{method_name} - name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{const_name} - _read_attribute(name) { |n| missing_attribute(n, caller) } - end - EOMETHOD - end - }.new - - extend ActiveSupport::Concern - - module ClassMethods - protected - def define_method_attribute(name) safe_name = name.unpack('h*'.freeze).first temp_method = "__temp__#{safe_name}" diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index bbf2a51a0e..5599b590ca 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -1,19 +1,6 @@ module ActiveRecord module AttributeMethods module Write - WriterMethodCache = Class.new(AttributeMethodCache) { - private - - def method_body(method_name, const_name) - <<-EOMETHOD - def #{method_name}(value) - name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{const_name} - write_attribute(name, value) - end - EOMETHOD - end - }.new - extend ActiveSupport::Concern included do diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index 854f9776a3..1f1b11eb68 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -179,7 +179,7 @@ module ActiveRecord # # If the +before_validation+ callback throws +:abort+, the process will be # aborted and {ActiveRecord::Base#save}[rdoc-ref:Persistence#save] will return +false+. - # If {ActiveRecord::Base#save!}[rdoc-ref:Persistence#save!] is called it will raise a ActiveRecord::RecordInvalid exception. + # If {ActiveRecord::Base#save!}[rdoc-ref:Persistence#save!] is called it will raise an ActiveRecord::RecordInvalid exception. # Nothing will be appended to the errors object. # # == Canceling callbacks diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 7e0c9f7837..bb5119d64e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -27,10 +27,10 @@ module ActiveRecord end # Returns an ActiveRecord::Result instance. - def select_all(arel, name = nil, binds = []) + def select_all(arel, name = nil, binds = [], preparable: nil) arel, binds = binds_from_relation arel, binds sql = to_sql(arel, binds) - if arel.is_a?(String) + if arel.is_a?(String) && preparable.nil? preparable = false else preparable = visitor.preparable diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 5e27cfe507..33dbab41cb 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -61,11 +61,11 @@ module ActiveRecord @query_cache.clear end - def select_all(arel, name = nil, binds = []) + def select_all(arel, name = nil, binds = [], preparable: nil) if @query_cache_enabled && !locked?(arel) arel, binds = binds_from_relation arel, binds sql = to_sql(arel, binds) - cache_sql(sql, binds) { super(sql, name, binds) } + cache_sql(sql, binds) { super(sql, name, binds, preparable: visitor.preparable) } else super end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index cb10ca9929..4f97c7c065 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -212,7 +212,7 @@ module ActiveRecord def initialize(name, temporary, options, as = nil) @columns_hash = {} @indexes = {} - @foreign_keys = {} + @foreign_keys = [] @primary_keys = nil @temporary = temporary @options = options @@ -330,7 +330,7 @@ module ActiveRecord end def foreign_key(table_name, options = {}) # :nodoc: - foreign_keys[table_name] = options + foreign_keys.push([table_name, options]) end # Appends <tt>:datetime</tt> columns <tt>:created_at</tt> and diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 1f429cfd94..de5b42e987 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -1,7 +1,7 @@ module ActiveRecord module Querying delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?, :any?, :many?, to: :all - delegate :second, :second!, :third, :third!, :fourth, :fourth!, :fifth, :fifth!, :forty_two, :forty_two!, to: :all + delegate :second, :second!, :third, :third!, :fourth, :fourth!, :fifth, :fifth!, :forty_two, :forty_two!, :third_to_last, :third_to_last!, :second_to_last, :second_to_last!, to: :all delegate :first_or_create, :first_or_create!, :first_or_initialize, to: :all delegate :find_or_create_by, :find_or_create_by!, :find_or_initialize_by, to: :all delegate :find_by, :find_by!, to: :all @@ -35,8 +35,8 @@ module ActiveRecord # # Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date] # Post.find_by_sql ["SELECT body FROM comments WHERE author = :user_id OR approved_by = :user_id", { :user_id => user_id }] - def find_by_sql(sql, binds = []) - result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds) + def find_by_sql(sql, binds = [], preparable: nil) + result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds, preparable: preparable) column_types = result_set.column_types.dup columns_hash.each_key { |k| column_types.delete k } message_bus = ActiveSupport::Notifications.instrumenter diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 99c0e71f97..956fe7c51e 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -239,6 +239,10 @@ module ActiveRecord def alias_candidate(name) "#{plural_name}_#{name}" end + + def chain + collect_join_chain + end end # Base class for AggregateReflection and AssociationReflection. Objects of @@ -421,7 +425,7 @@ module ActiveRecord # A chain of reflections from this one back to the owner. For more see the explanation in # ThroughReflection. - def chain + def collect_join_chain [self] end @@ -495,6 +499,18 @@ module ActiveRecord VALID_AUTOMATIC_INVERSE_MACROS = [:has_many, :has_one, :belongs_to] INVALID_AUTOMATIC_INVERSE_OPTIONS = [:conditions, :through, :polymorphic, :foreign_key] + def add_as_source(seed) + seed + end + + def add_as_polymorphic_through(reflection, seed) + seed + [PolymorphicReflection.new(self, reflection)] + end + + def add_as_through(seed) + seed + [self] + end + protected def actual_source_reflection # FIXME: this is a horrible name @@ -742,19 +758,8 @@ module ActiveRecord # # => [<ActiveRecord::Reflection::ThroughReflection: @delegate_reflection=#<ActiveRecord::Reflection::HasManyReflection: @name=:tags...>, # <ActiveRecord::Reflection::HasManyReflection: @name=:taggings, @options={}, @active_record=Post>] # - def chain - @chain ||= begin - a = source_reflection.chain - b = through_reflection.chain.map(&:dup) - - if options[:source_type] - b[0] = PolymorphicReflection.new(b[0], self) - end - - chain = a + b - chain[0] = self # Use self so we don't lose the information from :source_type - chain - end + def collect_join_chain + collect_join_reflections [self] end # This is for clearing cache on the reflection. Useful for tests that need to compare @@ -913,6 +918,27 @@ module ActiveRecord scope_chain end + def add_as_source(seed) + collect_join_reflections seed + end + + def add_as_polymorphic_through(reflection, seed) + collect_join_reflections(seed + [PolymorphicReflection.new(self, reflection)]) + end + + def add_as_through(seed) + collect_join_reflections(seed + [self]) + end + + def collect_join_reflections(seed) + a = source_reflection.add_as_source seed + if options[:source_type] + through_reflection.add_as_polymorphic_through self, a + else + through_reflection.add_as_through a + end + end + protected def actual_source_reflection # FIXME: this is a horrible name diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index d48bcea28a..16563515f6 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -145,15 +145,21 @@ module ActiveRecord # # [#<Person id:4>, #<Person id:3>, #<Person id:2>] def last(limit = nil) - if limit - if order_values.empty? && primary_key - order(arel_attribute(primary_key).desc).limit(limit).reverse - else - to_a.last(limit) - end - else - find_last - end + return find_last(limit) if loaded? || limit_value + + result = limit(limit || 1) + result.order!(arel_attribute(primary_key)) if order_values.empty? && primary_key + result = result.reverse_order! + + limit ? result.reverse : result.first + rescue ActiveRecord::IrreversibleOrderError + ActiveSupport::Deprecation.warn(<<-WARNING.squish) + Finding a last element by loading the relation when SQL ORDER + can not be reversed is deprecated. + Rails 5.1 will raise ActiveRecord::IrreversibleOrderError in this case. + Please call `to_a.last` if you still want to load the relation. + WARNING + find_last(limit) end # Same as #last but raises ActiveRecord::RecordNotFound if no record @@ -242,6 +248,38 @@ module ActiveRecord find_nth! 41 end + # Find the third-to-last record. + # If no order is defined it will order by primary key. + # + # Person.third_to_last # returns the third-to-last object fetched by SELECT * FROM people + # Person.offset(3).third_to_last # returns the third-to-last object from OFFSET 3 + # Person.where(["user_name = :u", { u: user_name }]).third_to_last + def third_to_last + find_nth(-3) + end + + # Same as #third_to_last but raises ActiveRecord::RecordNotFound if no record + # is found. + def third_to_last! + find_nth!(-3) + end + + # Find the second-to-last record. + # If no order is defined it will order by primary key. + # + # Person.second_to_last # returns the second-to-last object fetched by SELECT * FROM people + # Person.offset(3).second_to_last # returns the second-to-last object from OFFSET 3 + # Person.where(["user_name = :u", { u: user_name }]).second_to_last + def second_to_last + find_nth(-2) + end + + # Same as #second_to_last but raises ActiveRecord::RecordNotFound if no record + # is found. + def second_to_last! + find_nth!(-2) + end + # Returns true if a record exists in the table that matches the +id+ or # conditions given, or false otherwise. The argument can take six forms: # @@ -298,7 +336,7 @@ module ActiveRecord end # This method is called whenever no records are found with either a single - # id or multiple ids and raises a ActiveRecord::RecordNotFound exception. + # id or multiple ids and raises an ActiveRecord::RecordNotFound exception. # # The error message is different depending on whether a single id or # multiple ids are provided. If multiple ids are provided, then the number @@ -523,19 +561,6 @@ module ActiveRecord relation.limit(limit).to_a end - def find_last - if loaded? - @records.last - else - @last ||= - if limit_value - to_a.last - else - reverse_order.limit(1).to_a.first - end - end - end - private def find_nth_with_limit_and_offset(index, limit, offset:) # :nodoc: @@ -546,5 +571,9 @@ module ActiveRecord find_nth_with_limit(index, limit) end end + + def find_last(limit) + limit ? to_a.last(limit) : to_a.last + end end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 91bfa4d131..4533f3263f 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -655,6 +655,10 @@ module ActiveRecord # # SELECT `posts`.* FROM `posts` WHERE (('id = 1' OR 'author_id = 3')) # def or(other) + unless other.is_a? Relation + raise ArgumentError, "You have passed #{other.class.name} object to #or. Pass an ActiveRecord::Relation object instead." + end + spawn.or!(other) end @@ -1112,6 +1116,8 @@ module ActiveRecord order_query.flat_map do |o| case o + when Arel::Attribute + o.desc when Arel::Nodes::Ordering o.reverse when String diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 2bfc5ff7ae..a9e1fd0dad 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -60,7 +60,7 @@ module ActiveRecord end # Accepts an array, or string of SQL conditions and sanitizes - # them into a valid SQL fragment for a ORDER clause. + # them into a valid SQL fragment for an ORDER clause. # # sanitize_sql_for_order(["field(id, ?)", [1,3,2]]) # # => "field(id, 1,3,2)" diff --git a/activerecord/lib/active_record/statement_cache.rb b/activerecord/lib/active_record/statement_cache.rb index f6b0efb88a..6c896ccea6 100644 --- a/activerecord/lib/active_record/statement_cache.rb +++ b/activerecord/lib/active_record/statement_cache.rb @@ -106,7 +106,7 @@ module ActiveRecord sql = query_builder.sql_for bind_values, connection - klass.find_by_sql sql, bind_values + klass.find_by_sql(sql, bind_values, preparable: true) end alias :call :execute end diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 6677e6dc5f..ecaf04e39e 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -45,7 +45,7 @@ module ActiveRecord end # Attempts to save the record just like {ActiveRecord::Base#save}[rdoc-ref:Base#save] but - # will raise a ActiveRecord::RecordInvalid exception instead of returning +false+ if the record is not valid. + # will raise an ActiveRecord::RecordInvalid exception instead of returning +false+ if the record is not valid. def save!(options={}) perform_validations(options) ? super : raise_validation_error end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index a376e2a17f..f0aa4521b5 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -19,7 +19,7 @@ module ActiveRecord relation = build_relation(finder_class, table, attribute, value) if record.persisted? && finder_class.primary_key.to_s != attribute.to_s if finder_class.primary_key - relation = relation.where.not(finder_class.primary_key => record.id) + relation = relation.where.not(finder_class.primary_key => record.id_was) else raise UnknownPrimaryKey.new(finder_class, "Can not validate uniqueness for persisted record without primary key.") end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index ecaa521283..e975f4fbdd 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -408,6 +408,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end assert_no_queries do + bulbs.third_to_last() + bulbs.third_to_last({}) + end + + assert_no_queries do + bulbs.second_to_last() + bulbs.second_to_last({}) + end + + assert_no_queries do bulbs.last() bulbs.last({}) end diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 3602ee7ba2..84aac3e721 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -108,7 +108,7 @@ class EachTest < ActiveRecord::TestCase end end - def test_find_in_batches_should_finish_the_end_option + def test_find_in_batches_should_end_at_the_finish_option assert_queries(6) do Post.find_in_batches(batch_size: 1, finish: 5) do |batch| assert_kind_of Array, batch @@ -316,7 +316,7 @@ class EachTest < ActiveRecord::TestCase end end - def test_in_batches_should_finish_the_end_option + def test_in_batches_should_end_at_the_finish_option post = Post.order('id DESC').where('id <= ?', 5).first assert_queries(7) do relation = Post.in_batches(of: 1, finish: 5, load: true).reverse_each.first diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 75a74c052d..5b41630cd8 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -101,7 +101,7 @@ class FinderTest < ActiveRecord::TestCase def test_find_with_ids_where_and_limit # Please note that Topic 1 is the only not approved so - # if it were among the first 3 it would raise a ActiveRecord::RecordNotFound + # if it were among the first 3 it would raise an ActiveRecord::RecordNotFound records = Topic.where(approved: true).limit(3).find([3,2,5,1,4]) assert_equal 3, records.size assert_equal 'The Third Topic of the day', records[0].title @@ -516,16 +516,44 @@ class FinderTest < ActiveRecord::TestCase assert_equal Topic.order("title").to_a.last(2), Topic.order("title").last(2) end - def test_last_with_integer_and_order_should_not_use_sql_limit - query = assert_sql { Topic.order("title").last(5).entries } - assert_equal 1, query.length - assert_no_match(/LIMIT/, query.first) + def test_last_with_integer_and_order_should_use_sql_limit + relation = Topic.order("title") + assert_queries(1) { relation.last(5) } + assert !relation.loaded? end - def test_last_with_integer_and_reorder_should_not_use_sql_limit - query = assert_sql { Topic.reorder("title").last(5).entries } - assert_equal 1, query.length - assert_no_match(/LIMIT/, query.first) + def test_last_with_integer_and_reorder_should_use_sql_limit + relation = Topic.reorder("title") + assert_queries(1) { relation.last(5) } + assert !relation.loaded? + end + + def test_last_on_loaded_relation_should_not_use_sql + relation = Topic.limit(10).load + assert_no_queries do + relation.last + relation.last(2) + end + end + + def test_last_with_irreversible_order + assert_deprecated do + Topic.order("coalesce(author_name, title)").last + end + end + + def test_last_on_relation_with_limit_and_offset + post = posts('sti_comments') + + comments = post.comments.order(id: :asc) + assert_equal comments.limit(2).to_a.last, comments.limit(2).last + assert_equal comments.limit(2).to_a.last(2), comments.limit(2).last(2) + assert_equal comments.limit(2).to_a.last(3), comments.limit(2).last(3) + + comments = comments.offset(1) + assert_equal comments.limit(2).to_a.last, comments.limit(2).last + assert_equal comments.limit(2).to_a.last(2), comments.limit(2).last(2) + assert_equal comments.limit(2).to_a.last(3), comments.limit(2).last(3) end def test_take_and_first_and_last_with_integer_should_return_an_array diff --git a/activerecord/test/cases/migration/references_foreign_key_test.rb b/activerecord/test/cases/migration/references_foreign_key_test.rb index b01415afb2..85435f4dbc 100644 --- a/activerecord/test/cases/migration/references_foreign_key_test.rb +++ b/activerecord/test/cases/migration/references_foreign_key_test.rb @@ -144,6 +144,22 @@ module ActiveRecord @connection.drop_table "testing", if_exists: true end end + + test "multiple foreign keys can be added to the same table" do + @connection.create_table :testings do |t| + t.integer :col_1 + t.integer :col_2 + + t.foreign_key :testing_parents, column: :col_1 + t.foreign_key :testing_parents, column: :col_2 + end + + fks = @connection.foreign_keys("testings") + + fk_definitions = fks.map {|fk| [fk.from_table, fk.to_table, fk.column] } + assert_equal([["testings", "testing_parents", "col_1"], + ["testings", "testing_parents", "col_2"]], fk_definitions) + end end end end diff --git a/activerecord/test/cases/relation/or_test.rb b/activerecord/test/cases/relation/or_test.rb index 28a0862f91..ce8c5ca489 100644 --- a/activerecord/test/cases/relation/or_test.rb +++ b/activerecord/test/cases/relation/or_test.rb @@ -82,5 +82,11 @@ module ActiveRecord assert_equal p.loaded?, true assert_equal expected, p.or(Post.where('id = 2')).to_a end + + def test_or_with_non_relation_object_raises_error + assert_raises ArgumentError do + Post.where(id: [1, 2, 3]).or(title: 'Rails') + end + end end end diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index ad5ca70f36..c918cbdef5 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -374,6 +374,18 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal 10, DeveloperCalledJamis.unscoped { DeveloperCalledJamis.poor }.length end + def test_default_scope_with_joins + assert_equal Comment.where(post_id: SpecialPostWithDefaultScope.pluck(:id)).count, + Comment.joins(:special_post_with_default_scope).count + assert_equal Comment.where(post_id: Post.pluck(:id)).count, + Comment.joins(:post).count + end + + def test_unscoped_with_joins_should_not_have_default_scope + assert_equal SpecialPostWithDefaultScope.unscoped { Comment.joins(:special_post_with_default_scope).to_a }, + Comment.joins(:post).to_a + end + def test_default_scope_select_ignored_by_aggregations assert_equal DeveloperWithSelect.all.to_a.count, DeveloperWithSelect.count end diff --git a/activerecord/test/cases/store_test.rb b/activerecord/test/cases/store_test.rb index ab63f5825c..bce86875e1 100644 --- a/activerecord/test/cases/store_test.rb +++ b/activerecord/test/cases/store_test.rb @@ -177,6 +177,7 @@ class StoreTest < ActiveRecord::TestCase assert_equal [:color], first_model.stored_attributes[:data] assert_equal [:color, :width, :height], second_model.stored_attributes[:data] assert_equal [:color, :area, :volume], third_model.stored_attributes[:data] + assert_equal [:color], first_model.stored_attributes[:data] end test "YAML coder initializes the store when a Nil value is given" do diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index 7502a55391..e601c53dbf 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -469,4 +469,15 @@ class UniquenessValidationTest < ActiveRecord::TestCase assert_match(/\AUnknown primary key for table dashboards in model/, e.message) assert_match(/Can not validate uniqueness for persisted record without primary key.\z/, e.message) end + + def test_validate_uniqueness_ignores_itself_when_primary_key_changed + Topic.validates_uniqueness_of(:title) + + t = Topic.new("title" => "This is a unique title") + assert t.save, "Should save t as unique" + + t.id += 1 + assert t.valid?, "Should be valid" + assert t.save, "Should still save t as unique" + end end diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index b38b17e90e..dcc5c5a310 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -14,6 +14,7 @@ class Comment < ActiveRecord::Base has_many :ratings belongs_to :first_post, :foreign_key => :post_id + belongs_to :special_post_with_default_scope, foreign_key: :post_id has_many :children, :class_name => 'Comment', :foreign_key => :parent_id belongs_to :parent, :class_name => 'Comment', :counter_cache => :children_count diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index bd333da081..f4c324803c 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,4 +1,16 @@ +* Add `#on_weekday?` method to `Date`, `Time`, and `DateTime`. + + `#on_weekday?` returns `true` if the receiving date/time does not fall on a Saturday + or Sunday. + + *Vipul A M* + +* Add `Array#second_to_last` and `Array#third_to_last` methods. + + *Brian Christian* + * Fix regression in `Hash#dig` for HashWithIndifferentAccess. + *Jon Moss* ## Rails 5.0.0.beta2 (February 01, 2016) ## diff --git a/activesupport/lib/active_support/core_ext/array/access.rb b/activesupport/lib/active_support/core_ext/array/access.rb index 3177d8498e..37d833887a 100644 --- a/activesupport/lib/active_support/core_ext/array/access.rb +++ b/activesupport/lib/active_support/core_ext/array/access.rb @@ -73,4 +73,18 @@ class Array def forty_two self[41] end + + # Equal to <tt>self[-3]</tt>. + # + # %w( a b c d e ).third_to_last # => "c" + def third_to_last + self[-3] + end + + # Equal to <tt>self[-2]</tt>. + # + # %w( a b c d e ).second_to_last # => "d" + def second_to_last + self[-2] + end end diff --git a/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb b/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb index e079af594d..2de0d19a7e 100644 --- a/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/date_and_time/calculations.rb @@ -51,6 +51,11 @@ module DateAndTime WEEKEND_DAYS.include?(wday) end + # Returns true if the date/time does not fall on a Saturday or Sunday. + def on_weekday? + !WEEKEND_DAYS.include?(wday) + end + # Returns a new date/time the specified number of days ago. def days_ago(days) advance(:days => -days) diff --git a/activesupport/lib/active_support/deprecation/behaviors.rb b/activesupport/lib/active_support/deprecation/behaviors.rb index 0de891f1a2..dc24e2d0e1 100644 --- a/activesupport/lib/active_support/deprecation/behaviors.rb +++ b/activesupport/lib/active_support/deprecation/behaviors.rb @@ -11,7 +11,7 @@ module ActiveSupport DEFAULT_BEHAVIORS = { raise: ->(message, callstack) { e = DeprecationException.new(message) - e.set_backtrace(callstack) + e.set_backtrace(callstack.map(&:to_s)) raise e }, diff --git a/activesupport/test/core_ext/array/access_test.rb b/activesupport/test/core_ext/array/access_test.rb index 3f1e0c4cb4..1d834667f0 100644 --- a/activesupport/test/core_ext/array/access_test.rb +++ b/activesupport/test/core_ext/array/access_test.rb @@ -26,6 +26,8 @@ class AccessTest < ActiveSupport::TestCase assert_equal array[3], array.fourth assert_equal array[4], array.fifth assert_equal array[41], array.forty_two + assert_equal array[-3], array.third_to_last + assert_equal array[-2], array.second_to_last end def test_without diff --git a/activesupport/test/core_ext/date_and_time_behavior.rb b/activesupport/test/core_ext/date_and_time_behavior.rb index 784547bdf8..54df87def8 100644 --- a/activesupport/test/core_ext/date_and_time_behavior.rb +++ b/activesupport/test/core_ext/date_and_time_behavior.rb @@ -301,6 +301,16 @@ module DateAndTimeBehavior assert_not date_time_init(2015,1,5,15,15,10).on_weekend? end + def test_on_weekday_on_sunday + assert_not date_time_init(2015,1,4,0,0,0).on_weekday? + assert_not date_time_init(2015,1,4,15,15,10).on_weekday? + end + + def test_on_weekday_on_monday + assert date_time_init(2015,1,5,0,0,0).on_weekday? + assert date_time_init(2015,1,5,15,15,10).on_weekday? + end + def with_bw_default(bw = :monday) old_bw = Date.beginning_of_week Date.beginning_of_week = bw diff --git a/activesupport/test/deprecation_test.rb b/activesupport/test/deprecation_test.rb index 58a0a3964d..45c88b79cb 100644 --- a/activesupport/test/deprecation_test.rb +++ b/activesupport/test/deprecation_test.rb @@ -105,13 +105,13 @@ class DeprecationTest < ActiveSupport::TestCase ActiveSupport::Deprecation.behavior = :raise message = 'Revise this deprecated stuff now!' - callstack = %w(foo bar baz) + callstack = caller_locations e = assert_raise ActiveSupport::DeprecationException do ActiveSupport::Deprecation.behavior.first.call(message, callstack) end assert_equal message, e.message - assert_equal callstack, e.backtrace + assert_equal callstack.map(&:to_s), e.backtrace.map(&:to_s) end def test_default_stderr_behavior diff --git a/guides/source/4_2_release_notes.md b/guides/source/4_2_release_notes.md index 8a59007420..73e6c2c05b 100644 --- a/guides/source/4_2_release_notes.md +++ b/guides/source/4_2_release_notes.md @@ -405,7 +405,7 @@ Please refer to the [Changelog][railties] for detailed changes. url: http://localhost:3001 namespace: my_app_development - # config/production.rb + # config/environments/production.rb Rails.application.configure do config.middleware.use ExceptionNotifier, config_for(:exception_notification) end diff --git a/guides/source/5_0_release_notes.md b/guides/source/5_0_release_notes.md index 4e8252f85b..d380ba8fce 100644 --- a/guides/source/5_0_release_notes.md +++ b/guides/source/5_0_release_notes.md @@ -705,7 +705,7 @@ Please refer to the [Changelog][active-support] for detailed changes. * Changed the default test order from `:sorted` to `:random`. ([commit](https://github.com/rails/rails/commit/5f777e4b5ee2e3e8e6fd0e2a208ec2a4d25a960d)) -* Added `#on_weekend?`, `#next_weekday`, `#prev_weekday` methods to `Date`, +* Added `#on_weekend?`, `#on_weekday?`, `#next_weekday`, `#prev_weekday` methods to `Date`, `Time`, and `DateTime`. ([Pull Request](https://github.com/rails/rails/pull/18335)) @@ -768,6 +768,7 @@ framework it is. Kudos to all of them. [action-pack]: https://github.com/rails/rails/blob/5-0-stable/actionpack/CHANGELOG.md [action-view]: https://github.com/rails/rails/blob/5-0-stable/actionview/CHANGELOG.md [action-mailer]: https://github.com/rails/rails/blob/5-0-stable/actionmailer/CHANGELOG.md +[action-cable]: https://github.com/rails/rails/blob/5-0-stable/actioncable/CHANGELOG.md [active-record]: https://github.com/rails/rails/blob/5-0-stable/activerecord/CHANGELOG.md [active-model]: https://github.com/rails/rails/blob/5-0-stable/activemodel/CHANGELOG.md [active-support]: https://github.com/rails/rails/blob/5-0-stable/activesupport/CHANGELOG.md diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md index cd2c13e8c1..91ea4efb55 100644 --- a/guides/source/action_mailer_basics.md +++ b/guides/source/action_mailer_basics.md @@ -222,7 +222,7 @@ class SendWeeklySummary end ``` -The method `welcome_email` returns a `ActionMailer::MessageDelivery` object which +The method `welcome_email` returns an `ActionMailer::MessageDelivery` object which can then just be told `deliver_now` or `deliver_later` to send itself out. The `ActionMailer::MessageDelivery` object is just a wrapper around a `Mail::Message`. If you want to inspect, alter or do anything else with the `Mail::Message` object you can @@ -278,7 +278,7 @@ different, encode your content and pass in the encoded content and encoding in a ```ruby encoded_content = SpecialEncode(File.read('/path/to/filename.jpg')) attachments['filename.jpg'] = { - mime_type: 'application/x-gzip', + mime_type: 'application/gzip', encoding: 'SpecialEncoding', content: encoded_content } diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md index 543937f8e5..5e6eae1071 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -1524,7 +1524,7 @@ Localized Views Action View has the ability to render different templates depending on the current locale. -For example, suppose you have a `ArticlesController` with a show action. By default, calling this action will render `app/views/articles/show.html.erb`. But if you set `I18n.locale = :de`, then `app/views/articles/show.de.html.erb` will be rendered instead. If the localized template isn't present, the undecorated version will be used. This means you're not required to provide localized views for all cases, but they will be preferred and used if available. +For example, suppose you have an `ArticlesController` with a show action. By default, calling this action will render `app/views/articles/show.html.erb`. But if you set `I18n.locale = :de`, then `app/views/articles/show.de.html.erb` will be rendered instead. If the localized template isn't present, the undecorated version will be used. This means you're not required to provide localized views for all cases, but they will be preferred and used if available. You can use the same technique to localize the rescue files in your public directory. For example, setting `I18n.locale = :de` and creating `public/500.de.html` and `public/404.de.html` would allow you to have localized rescue pages. diff --git a/guides/source/active_model_basics.md b/guides/source/active_model_basics.md index c05e20aceb..a8199e5d02 100644 --- a/guides/source/active_model_basics.md +++ b/guides/source/active_model_basics.md @@ -319,7 +319,7 @@ person.serializable_hash # => {"name"=>"Bob"} #### ActiveModel::Serializers -Rails provides a `ActiveModel::Serializers::JSON` serializer. +Rails provides an `ActiveModel::Serializers::JSON` serializer. This module automatically include the `ActiveModel::Serialization`. ##### ActiveModel::Serializers::JSON diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index 83f4b951ee..bd7dbd0f11 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -883,7 +883,7 @@ Changing Existing Migrations ---------------------------- Occasionally you will make a mistake when writing a migration. If you have -already run the migration then you cannot just edit the migration and run the +already run the migration, then you cannot just edit the migration and run the migration again: Rails thinks it has already run the migration and so will do nothing when you run `rails db:migrate`. You must rollback the migration (for example with `bin/rails db:rollback`), edit your migration and then run @@ -933,7 +933,7 @@ There are two ways to dump the schema. This is set in `config/application.rb` by the `config.active_record.schema_format` setting, which may be either `:sql` or `:ruby`. -If `:ruby` is selected then the schema is stored in `db/schema.rb`. If you look +If `:ruby` is selected, then the schema is stored in `db/schema.rb`. If you look at this file you'll find that it looks an awful lot like one very big migration: diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 63658e7c8b..af15d4870c 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -11,7 +11,7 @@ After reading this guide, you will know: * How to specify the order, retrieved attributes, grouping, and other properties of the found records. * How to use eager loading to reduce the number of database queries needed for data retrieval. * How to use dynamic finder methods. -* How to use method chaining to use multiple ActiveRecord methods together. +* How to use method chaining to use multiple Active Record methods together. * How to check for the existence of particular records. * How to perform various calculations on Active Record models. * How to run EXPLAIN on relations. @@ -1296,6 +1296,28 @@ Using a class method is the preferred way to accept arguments for scopes. These category.articles.created_before(time) ``` +### Using conditionals + +Your scope can utilize conditionals: + +```ruby +class Article < ApplicationRecord + scope :created_before, ->(time) { where("created_at < ?", time) if time.present? } +end +``` + +Like the other examples, this will behave similarly to a class method. + +```ruby +class Article < ApplicationRecord + def self.created_before(time) + where("created_at < ?", time) if time.present? + end +end +``` + +However, there is one important caveat: A scope will always return an `ActiveRecord::Relation` object, even if the conditional evaluates to `false`, whereas a class method, will return `nil`. This can cause `NoMethodError` when chaining class methods with conditionals, if any of the conditionals return `false`. + ### Applying a default scope If we wish for a scope to be applied across all queries to the model we can use the diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index dd7adf09a2..10bd201145 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -149,7 +149,7 @@ false` as an argument. This technique should be used with caution. ### `valid?` and `invalid?` -Before saving an ActiveRecord object, Rails runs your validations. +Before saving an Active Record object, Rails runs your validations. If these validations produce any errors, Rails does not save the object. You can also run these validations on your own. `valid?` triggers your validations diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index 0aca6db9b6..e66b9a4301 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -2240,7 +2240,7 @@ Similarly, `from` returns the tail from the element at the passed index to the e [].from(0) # => [] ``` -The methods `second`, `third`, `fourth`, and `fifth` return the corresponding element (`first` is built-in). Thanks to social wisdom and positive constructiveness all around, `forty_two` is also available. +The methods `second`, `third`, `fourth`, and `fifth` return the corresponding element, as do `second_to_last` and `third_to_last` (`first` and `last` are built-in). Thanks to social wisdom and positive constructiveness all around, `forty_two` is also available. ```ruby %w(a b c d).third # => "c" @@ -3078,7 +3078,7 @@ INFO: The following calculation methods have edge cases in October 1582, since d #### `Date.current` -Active Support defines `Date.current` to be today in the current time zone. That's like `Date.today`, except that it honors the user time zone, if defined. It also defines `Date.yesterday` and `Date.tomorrow`, and the instance predicates `past?`, `today?`, and `future?`, all of them relative to `Date.current`. +Active Support defines `Date.current` to be today in the current time zone. That's like `Date.today`, except that it honors the user time zone, if defined. It also defines `Date.yesterday` and `Date.tomorrow`, and the instance predicates `past?`, `today?`, `future?`, `on_weekday?` and `on_weekend?`, all of them relative to `Date.current`. When making Date comparisons using methods which honor the user time zone, make sure to use `Date.current` and not `Date.today`. There are cases where the user time zone might be in the future compared to the system time zone, which `Date.today` uses by default. This means `Date.today` may equal `Date.yesterday`. @@ -3467,6 +3467,8 @@ years_ago years_since prev_year (last_year) next_year +on_weekday? +on_weekend? ``` The following methods are reimplemented so you do **not** need to load `active_support/core_ext/date/calculations.rb` for these ones: @@ -3653,6 +3655,8 @@ years_ago years_since prev_year (last_year) next_year +on_weekday? +on_weekend? ``` They are analogous. Please refer to their documentation above and take into account the following differences: diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index 4efffc6605..5dd54bf8ad 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -901,7 +901,7 @@ your CDN server, you need to tell browsers to use your CDN to grab assets instead of your Rails server directly. You can do this by configuring Rails to set your CDN as the asset host instead of using a relative path. To set your asset host in Rails, you need to set `config.action_controller.asset_host` in -`config/production.rb`: +`config/environments/production.rb`: ```ruby config.action_controller.asset_host = 'mycdnsubdomain.fictional-cdn.com' diff --git a/guides/source/association_basics.md b/guides/source/association_basics.md index 3386791cdb..09ab64837a 100644 --- a/guides/source/association_basics.md +++ b/guides/source/association_basics.md @@ -713,7 +713,7 @@ By default, Active Record doesn't know about the connection between these associ ```ruby a = Author.first -b = c.books.first +b = a.books.first a.first_name == b.author.first_name # => true a.first_name = 'Manny' a.first_name == b.author.first_name # => false @@ -735,7 +735,7 @@ With these changes, Active Record will only load one copy of the author object, ```ruby a = author.first -b = c.books.first +b = a.books.first a.first_name == b.author.first_name # => true a.first_name = 'Manny' a.first_name == b.author.first_name # => true diff --git a/guides/source/configuring.md b/guides/source/configuring.md index d9c345fb71..a5fb396f15 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -998,7 +998,7 @@ Below is a comprehensive list of all the initializers found in Rails in the orde * `initialize_cache` If `Rails.cache` isn't set yet, initializes the cache by referencing the value in `config.cache_store` and stores the outcome as `Rails.cache`. If this object responds to the `middleware` method, its middleware is inserted before `Rack::Runtime` in the middleware stack. -* `set_clear_dependencies_hook` Provides a hook for `active_record.set_dispatch_hooks` to use, which will run before this initializer. This initializer - which runs only if `cache_classes` is set to `false` - uses `ActionDispatch::Callbacks.after` to remove the constants which have been referenced during the request from the object space so that they will be reloaded during the following request. +* `set_clear_dependencies_hook` This initializer - which runs only if `cache_classes` is set to `false` - uses `ActionDispatch::Callbacks.after` to remove the constants which have been referenced during the request from the object space so that they will be reloaded during the following request. * `initialize_dependency_mechanism` If `config.cache_classes` is true, configures `ActiveSupport::Dependencies.mechanism` to `require` dependencies rather than `load` them. @@ -1012,13 +1012,17 @@ Below is a comprehensive list of all the initializers found in Rails in the orde * `active_support.initialize_beginning_of_week` Sets the default beginning of week for the application based on `config.beginning_of_week` setting, which defaults to `:monday`. +* `active_support.set_configs` Sets up Active Support by using the settings in `config.active_support` by `send`'ing the method names as setters to `ActiveSupport` and passing the values through. + * `action_dispatch.configure` Configures the `ActionDispatch::Http::URL.tld_length` to be set to the value of `config.action_dispatch.tld_length`. * `action_view.set_configs` Sets up Action View by using the settings in `config.action_view` by `send`'ing the method names as setters to `ActionView::Base` and passing the values through. -* `action_controller.logger` Sets `ActionController::Base.logger` - if it's not already set - to `Rails.logger`. +* `action_controller.assets_config` Initializes the `config.actions_controller.assets_dir` to the app's public directory if not explicitly configured + +* `action_controller.set_helpers_path` Sets Action Controller's helpers_path to the application's helpers_path -* `action_controller.initialize_framework_caches` Sets `ActionController::Base.cache_store` - if it's not already set - to `Rails.cache`. +* `action_controller.parameters_config` Configures strong parameters options for `ActionController::Parameters` * `action_controller.set_configs` Sets up Action Controller by using the settings in `config.action_controller` by `send`'ing the method names as setters to `ActionController::Base` and passing the values through. @@ -1028,13 +1032,21 @@ Below is a comprehensive list of all the initializers found in Rails in the orde * `active_record.logger` Sets `ActiveRecord::Base.logger` - if it's not already set - to `Rails.logger`. +* `active_record.migration_error` Configures middleware to check for pending migrations + +* `active_record.check_schema_cache_dump` Loads the schema cache dump if configured and available + +* `active_record.warn_on_records_fetched_greater_than` Enables warnings when queries return large numbers of records + * `active_record.set_configs` Sets up Active Record by using the settings in `config.active_record` by `send`'ing the method names as setters to `ActiveRecord::Base` and passing the values through. * `active_record.initialize_database` Loads the database configuration (by default) from `config/database.yml` and establishes a connection for the current environment. * `active_record.log_runtime` Includes `ActiveRecord::Railties::ControllerRuntime` which is responsible for reporting the time taken by Active Record calls for the request back to the logger. -* `active_record.set_dispatch_hooks` Resets all reloadable connections to the database if `config.cache_classes` is set to `false`. +* `active_record.set_reloader_hooks` Resets all reloadable connections to the database if `config.cache_classes` is set to `false`. + +* `active_record.add_watchable_files` Adds `schema.rb` and `structure.sql` files to watchable files * `active_job.logger` Sets `ActiveJob::Base.logger` - if it's not already set - to `Rails.logger`. diff --git a/guides/source/engines.md b/guides/source/engines.md index 415def8367..db50ad278f 100644 --- a/guides/source/engines.md +++ b/guides/source/engines.md @@ -461,7 +461,7 @@ model, a comment controller and then modify the articles scaffold to display comments and allow people to create new ones. From the application root, run the model generator. Tell it to generate a -`Comment` model, with the related table having two columns: a `article_id` integer +`Comment` model, with the related table having two columns: an `article_id` integer and `text` text column. ```bash diff --git a/guides/source/i18n.md b/guides/source/i18n.md index 5bbd4048b9..56b0c6c812 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -25,7 +25,7 @@ After reading this guide, you will know: * How I18n works in Ruby on Rails * How to correctly use I18n into a RESTful application in various ways -* How to use I18n to translate ActiveRecord errors or ActionMailer E-mail subjects +* How to use I18n to translate Active Record errors or Action Mailer E-mail subjects * Some other tools to go further with the translation process of your application -------------------------------------------------------------------------------- diff --git a/guides/source/layout.html.erb b/guides/source/layout.html.erb index 1f81ea4694..6db76b528e 100644 --- a/guides/source/layout.html.erb +++ b/guides/source/layout.html.erb @@ -24,7 +24,17 @@ <% end %> <div id="topNav"> <div class="wrapper"> - <strong class="more-info-label">←<a href="http://rubyonrails.org/">Back to rubyonrails.org:</a> </strong> + <strong class="more-info-label">More at <a href="http://rubyonrails.org/">rubyonrails.org:</a> </strong> + <span class="red-button more-info-button"> + More Ruby on Rails + </span> + <ul class="more-info-links s-hidden"> + <li class="more-info"><a href="http://weblog.rubyonrails.org/">Blog</a></li> + <li class="more-info"><a href="http://guides.rubyonrails.org/">Guides</a></li> + <li class="more-info"><a href="http://api.rubyonrails.org/">API</a></li> + <li class="more-info"><a href="http://stackoverflow.com/questions/tagged/ruby-on-rails">Ask for help</a></li> + <li class="more-info"><a href="https://github.com/rails/rails">Contribute on GitHub</a></li> + </ul> </div> </div> <div id="header"> diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md index d55e1007ee..83173e8d75 100644 --- a/guides/source/layouts_and_rendering.md +++ b/guides/source/layouts_and_rendering.md @@ -555,7 +555,7 @@ class Admin::ProductsController < AdminController end ``` -The lookup order for a `admin/products#index` action will be: +The lookup order for an `admin/products#index` action will be: * `app/views/admin/products/` * `app/views/admin/` @@ -700,7 +700,7 @@ This would detect that there are no books with the specified ID, populate the `@ ### Using `head` To Build Header-Only Responses -The `head` method can be used to send responses with only headers to the browser. It provides a more obvious alternative to calling `render :nothing`. The `head` method accepts a number or symbol (see [reference table](#the-status-option)) representing a HTTP status code. The options argument is interpreted as a hash of header names and values. For example, you can return only an error header: +The `head` method can be used to send responses with only headers to the browser. The `head` method accepts a number or symbol (see [reference table](#the-status-option)) representing a HTTP status code. The options argument is interpreted as a hash of header names and values. For example, you can return only an error header: ```ruby head :bad_request diff --git a/guides/source/routing.md b/guides/source/routing.md index 777d1d24b6..bd3e236a2b 100644 --- a/guides/source/routing.md +++ b/guides/source/routing.md @@ -1136,19 +1136,19 @@ For example, here's a small section of the `rails routes` output for a RESTful r edit_user GET /users/:id/edit(.:format) users#edit ``` -You can search through your routes with the --grep option (-g for short). This outputs any routes that partially match the URL helper method name, the HTTP verb, or the URL path. +You can search through your routes with the grep option: -g. This outputs any routes that partially match the URL helper method name, the HTTP verb, or the URL path. ``` -$ bin/rails routes --grep new_comment +$ bin/rails routes -g new_comment $ bin/rails routes -g POST $ bin/rails routes -g admin ``` -If you only want to see the routes that map to a specific controller, there's the --controller option (-c for short). +If you only want to see the routes that map to a specific controller, there's the -c option. ``` -$ bin/rails routes --controller users -$ bin/rails routes --controller admin/users +$ bin/rails routes -c users +$ bin/rails routes -c admin/users $ bin/rails routes -c Comments $ bin/rails routes -c Articles::CommentsController ``` diff --git a/guides/source/security.md b/guides/source/security.md index b30bed3767..98324141cc 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -23,7 +23,7 @@ Web application frameworks are made to help developers build web applications. S In general there is no such thing as plug-n-play security. Security depends on the people using the framework, and sometimes on the development method. And it depends on all layers of a web application environment: The back-end storage, the web server and the web application itself (and possibly other layers or applications). -The Gartner Group however estimates that 75% of attacks are at the web application layer, and found out "that out of 300 audited sites, 97% are vulnerable to attack". This is because web applications are relatively easy to attack, as they are simple to understand and manipulate, even by the lay person. +The Gartner Group, however, estimates that 75% of attacks are at the web application layer, and found out "that out of 300 audited sites, 97% are vulnerable to attack". This is because web applications are relatively easy to attack, as they are simple to understand and manipulate, even by the lay person. The threats against web applications include user account hijacking, bypass of access control, reading or modifying sensitive data, or presenting fraudulent content. Or an attacker might be able to install a Trojan horse program or unsolicited e-mail sending software, aim at financial enrichment or cause brand name damage by modifying company resources. In order to prevent attacks, minimize their impact and remove points of attack, first of all, you have to fully understand the attack methods in order to find the correct countermeasures. That is what this guide aims at. @@ -62,7 +62,7 @@ Many web applications have an authentication system: a user provides a user name Hence, the cookie serves as temporary authentication for the web application. Anyone who seizes a cookie from someone else, may use the web application as this user - with possibly severe consequences. Here are some ways to hijack a session, and their countermeasures: -* Sniff the cookie in an insecure network. A wireless LAN can be an example of such a network. In an unencrypted wireless LAN it is especially easy to listen to the traffic of all connected clients. For the web application builder this means to _provide a secure connection over SSL_. In Rails 3.1 and later, this could be accomplished by always forcing SSL connection in your application config file: +* Sniff the cookie in an insecure network. A wireless LAN can be an example of such a network. In an unencrypted wireless LAN, it is especially easy to listen to the traffic of all connected clients. For the web application builder this means to _provide a secure connection over SSL_. In Rails 3.1 and later, this could be accomplished by always forcing SSL connection in your application config file: ```ruby config.force_ssl = true diff --git a/guides/source/testing.md b/guides/source/testing.md index 1c64b2c0ac..7a9a30f7ac 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -830,7 +830,7 @@ end If we run our test now, we should see a failure: ```bash -$ bin/rails test test/controllers/articles_controller_test.rb test_should_create_article +$ bin/rails test test/controllers/articles_controller_test.rb -n test_should_create_article Run options: -n test_should_create_article --seed 32266 # Running: @@ -868,7 +868,7 @@ end Now if we run our tests, we should see it pass: ```bash -$ bin/rails test test/controllers/articles_controller_test.rb test_should_create_article +$ bin/rails test test/controllers/articles_controller_test.rb -n test_should_create_article Run options: -n test_should_create_article --seed 18981 # Running: @@ -1191,9 +1191,9 @@ testing) but instead it will be appended to an array (`ActionMailer::Base.deliveries`). NOTE: The `ActionMailer::Base.deliveries` array is only reset automatically in -`ActionMailer::TestCase` tests. If you want to have a clean slate outside Action -Mailer tests, you can reset it manually with: -`ActionMailer::Base.deliveries.clear` +`ActionMailer::TestCase` and `ActionDispatch::IntegrationTest` tests. +If you want to have a clean slate outside these test cases, you can reset it +manually with: `ActionMailer::Base.deliveries.clear` ### Functional Testing diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index e631445492..0dfa4f1cb8 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -16,6 +16,21 @@ Before attempting to upgrade an existing application, you should be sure you hav The best way to be sure that your application still works after upgrading is to have good test coverage before you start the process. If you don't have automated tests that exercise the bulk of your application, you'll need to spend time manually exercising all the parts that have changed. In the case of a Rails upgrade, that will mean every single piece of functionality in the application. Do yourself a favor and make sure your test coverage is good _before_ you start an upgrade. +### The Upgrade Process + +When changing Rails versions, it's best to move slowly, one minor version at a time, in order to make good use of the deprecation warnings. Rails version numbers are in the form Major.Minor.Patch. Major and Minor versions are allowed to make changes to the public API, so this may cause errors in your application. Patch versions only include bug fixes, and don't change any public API. + +The process should go as follows: + +1. Write tests and make sure they pass +2. Move to the latest patch version after your current version +3. Fix tests and deprecated features +4. Move to the latest patch version of the next minor version + +Repeat this process until you reach your target Rails version. Each time you move versions, you will need to change the Rails version number in the Gemfile (and possibly other gem versions) and run `bundle update`. Then run the Update rake task mentioned below to update configuration files, then run your tests. + +You can find a list of all released Rails versions [here](https://rubygems.org/gems/rails/versions). + ### Ruby Versions Rails generally stays close to the latest released Ruby version when it's released: @@ -168,7 +183,7 @@ the logs. In the next version, these errors will no longer be suppressed. Instead, the errors will propagate normally just like in other Active Record callbacks. -When you define a `after_rollback` or `after_commit` callback, you +When you define an `after_rollback` or `after_commit` callback, you will receive a deprecation warning about this upcoming change. When you are ready, you can opt into the new behavior and remove the deprecation warning by adding following configuration to your diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 8f4dc736a8..a3be5356a1 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,21 @@ +* Change fail fast of `bin/rails test` interrupts run on error. + + *Yuji Yaginuma* + +* The application generator supports `--skip-listen` to opt-out of features + that depend on the listen gem. As of this writing they are the evented file + system monitor and the async plugin for spring. + +* The Gemfiles of new applications include spring-watcher-listen on Linux and + Mac OS X (unless --skip-spring). + + *Xavier Noria* + +* New applications are generated with the evented file system monitor enabled + on Linux and Mac OS X. + + *Xavier Noria* + * Add dummy files for apple-touch-icon.png and apple-touch-icon.png. GH#23427 *Alexey Zabelin* diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index cac31e1eed..bff33ff42e 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -214,7 +214,7 @@ module Rails # url: http://localhost:3001 # namespace: my_app_development # - # # config/production.rb + # # config/environments/production.rb # Rails.application.configure do # config.middleware.use ExceptionNotifier, config_for(:exception_notification) # end diff --git a/railties/lib/rails/engine/configuration.rb b/railties/lib/rails/engine/configuration.rb index 8cadbc3ddd..294d07446f 100644 --- a/railties/lib/rails/engine/configuration.rb +++ b/railties/lib/rails/engine/configuration.rb @@ -39,6 +39,7 @@ module Rails paths.add "app", eager_load: true, glob: "{*,*/concerns}" paths.add "app/assets", glob: "*" paths.add "app/controllers", eager_load: true + paths.add "app/channels", eager_load: true, glob: "**/*_channel.rb" paths.add "app/helpers", eager_load: true paths.add "app/models", eager_load: true paths.add "app/mailers", eager_load: true diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index e3d79521e7..330bd7ec5d 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -105,7 +105,7 @@ module Rails # Configure generators for API only applications. It basically hides # everything that is usually browser related, such as assets and session - # migration generators, and completely disable views, helpers and assets + # migration generators, and completely disable helpers and assets # so generators such as scaffold won't create them. def self.api_only! hide_namespaces "assets", "helper", "css", "js" @@ -116,6 +116,10 @@ module Rails helper: false, template_engine: nil ) + + if ARGV.first == 'mailer' + options[:rails].merge!(template_engine: :erb) + end end # Remove the color from output. diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 9b1e16a7a3..9adfcc6ee7 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -63,6 +63,9 @@ module Rails class_option :skip_spring, type: :boolean, default: false, desc: "Don't install Spring application preloader" + class_option :skip_listen, type: :boolean, default: false, + desc: "Don't generate configuration that depends on the listen gem" + class_option :skip_javascript, type: :boolean, aliases: '-J', default: false, desc: 'Skip JavaScript files' @@ -390,6 +393,14 @@ module Rails !options[:skip_spring] && !options.dev? && Process.respond_to?(:fork) && !RUBY_PLATFORM.include?("cygwin") end + def depend_on_listen? + !options[:skip_listen] && os_supports_listen_out_of_the_box? + end + + def os_supports_listen_out_of_the_box? + RbConfig::CONFIG['host_os'] =~ /darwin|linux/ + end + def run_bundle bundle_command('install') if bundle_install? end diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 248ad20019..885f0c20f6 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -276,9 +276,9 @@ module Rails end end - def delete_app_views_if_api_option + def delete_application_layout_file_if_api_option if options[:api] - remove_dir 'app/views' + remove_file 'app/views/layouts/application.html.erb' end end diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index 3825dc4e38..f3bc9d9734 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -38,9 +38,15 @@ group :development do gem 'web-console', '~> 3.0' <%- end -%> <%- end -%> +<% if depend_on_listen? -%> + gem 'listen', '~> 3.0.5' +<% end -%> <% if spring_install? -%> # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring gem 'spring' +<% if depend_on_listen? -%> + gem 'spring-watcher-listen', '~> 2.0.0' +<% end -%> <% end -%> end <% end -%> diff --git a/railties/lib/rails/generators/rails/app/templates/config/databases/postgresql.yml b/railties/lib/rails/generators/rails/app/templates/config/databases/postgresql.yml index d51b2ec199..bd5c0b10f6 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/databases/postgresql.yml +++ b/railties/lib/rails/generators/rails/app/templates/config/databases/postgresql.yml @@ -19,7 +19,7 @@ default: &default encoding: unicode # For details on connection pooling, see rails configuration guide # http://guides.rubyonrails.org/configuring.html#database-pooling - pool: 5 + pool: <%%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> development: <<: *default diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt index fd41372d9c..d4e2b1c756 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt @@ -58,5 +58,5 @@ Rails.application.configure do # Use an evented file watcher to asynchronously detect changes in source code, # routes, locales, etc. This feature depends on the listen gem. - # config.file_watcher = ActiveSupport::EventedFileUpdateChecker + <%= '# ' unless depend_on_listen? %>config.file_watcher = ActiveSupport::EventedFileUpdateChecker end diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt index 8133917591..e8c8b00669 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt @@ -35,9 +35,6 @@ Rails.application.configure do config.action_mailer.delivery_method = :test <%- end -%> - # Randomize the order test cases are executed. - config.active_support.test_order = :random - # Print deprecation notices to the stderr. config.active_support.deprecation = :stderr diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index b5e836b584..565764842b 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -287,10 +287,6 @@ task default: :test protected - def app_templates_dir - "../../app/templates" - end - def create_dummy_app(path = nil) dummy_path(path) if path diff --git a/railties/lib/rails/generators/rails/plugin/templates/app/controllers/%namespaced_name%/application_controller.rb.tt b/railties/lib/rails/generators/rails/plugin/templates/app/controllers/%namespaced_name%/application_controller.rb.tt index 83807f14b4..abbacd9bec 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/app/controllers/%namespaced_name%/application_controller.rb.tt +++ b/railties/lib/rails/generators/rails/plugin/templates/app/controllers/%namespaced_name%/application_controller.rb.tt @@ -1,6 +1,6 @@ <%= wrap_in_modules <<-rb.strip_heredoc class ApplicationController < ActionController::#{api? ? "API" : "Base"} - #{ api? ? '# ' : '' }protect_from_forgery :with => :exception + #{ api? ? '# ' : '' }protect_from_forgery with: :exception end rb %> diff --git a/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml b/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml index 2656767eb4..0681780c97 100644 --- a/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml +++ b/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml @@ -6,7 +6,7 @@ <%- if attribute.password_digest? -%> password_digest: <%%= BCrypt::Password.create('secret') %> <%- elsif attribute.reference? -%> - <%= yaml_key_value(attribute.column_name.sub(/_id$/, ''), attribute.default) %> + <%= yaml_key_value(attribute.column_name.sub(/_id$/, ''), attribute.default || name) %> <%- else -%> <%= yaml_key_value(attribute.column_name, attribute.default) %> <%- end -%> diff --git a/railties/lib/rails/railtie.rb b/railties/lib/rails/railtie.rb index 8c24d1d56d..99dd571a00 100644 --- a/railties/lib/rails/railtie.rb +++ b/railties/lib/rails/railtie.rb @@ -183,7 +183,7 @@ module Rails end protected - def generate_railtie_name(string) + def generate_railtie_name(string) #:nodoc: ActiveSupport::Inflector.underscore(string).tr("/", "_") end @@ -200,21 +200,24 @@ module Rails delegate :railtie_name, to: :class - def initialize + def initialize #:nodoc: if self.class.abstract_railtie? raise "#{self.class.name} is abstract, you cannot instantiate it directly." end end - def configure(&block) + def configure(&block) #:nodoc: instance_eval(&block) end + # This is used to create the <tt>config</tt> object on Railties, an instance of + # Railtie::Configuration, that is used by Railties and Application to store + # related configuration. def config @config ||= Railtie::Configuration.new end - def railtie_namespace + def railtie_namespace #:nodoc: @railtie_namespace ||= self.class.parents.detect { |n| n.respond_to?(:railtie_namespace) } end diff --git a/railties/lib/rails/tasks/routes.rake b/railties/lib/rails/tasks/routes.rake index 939fa59c75..69103aa5d9 100644 --- a/railties/lib/rails/tasks/routes.rake +++ b/railties/lib/rails/tasks/routes.rake @@ -2,7 +2,7 @@ require 'active_support/deprecation' require 'active_support/core_ext/string/strip' # for strip_heredoc require 'optparse' -desc 'Print out all defined routes in match order, with names. Target specific controller with --controller option - or its -c shorthand.' +desc 'Print out all defined routes in match order, with names. Target specific controller with -c option, or grep routes using -g option' task routes: :environment do all_routes = Rails.application.routes.routes require 'action_dispatch/routing/inspector' @@ -19,11 +19,11 @@ task routes: :environment do OptionParser.new do |opts| opts.banner = "Usage: rails routes [options]" - opts.on("-c", "--controller [CONTROLLER]") do |controller| + opts.on("-c CONTROLLER") do |controller| routes_filter = { controller: controller } end - opts.on("-g", "--grep [PATTERN]") do |pattern| + opts.on("-g PATTERN") do |pattern| routes_filter = pattern end diff --git a/railties/lib/rails/test_unit/minitest_plugin.rb b/railties/lib/rails/test_unit/minitest_plugin.rb index 29a3d991b8..03c705ffef 100644 --- a/railties/lib/rails/test_unit/minitest_plugin.rb +++ b/railties/lib/rails/test_unit/minitest_plugin.rb @@ -42,7 +42,7 @@ module Minitest end opts.on("-f", "--fail-fast", - "Abort test run on first failure") do + "Abort test run on first failure or error") do options[:fail_fast] = true end diff --git a/railties/lib/rails/test_unit/reporter.rb b/railties/lib/rails/test_unit/reporter.rb index 73b8d7d27b..e81bd9df7e 100644 --- a/railties/lib/rails/test_unit/reporter.rb +++ b/railties/lib/rails/test_unit/reporter.rb @@ -24,7 +24,7 @@ module Rails io.puts end - if fail_fast? && result.failure && !result.error? && !result.skipped? + if fail_fast? && result.failure && !result.skipped? raise Interrupt end end @@ -68,19 +68,13 @@ module Rails def format_failures(result) result.failures.map do |failure| - "#{failure.result_label}:\n#{result.class}##{result.name}:\n#{failure.message}\n" + "#{failure.result_label}:\n#{result.location}:\n#{failure.message}\n" end end def format_rerun_snippet(result) - # Try to extract path to assertion from backtrace. - if result.location =~ /\[(.*)\]\z/ - assertion_path = $1 - else - assertion_path = result.method(result.name).source_location.join(':') - end - - "#{self.executable} #{relative_path_for(assertion_path)}" + location, line = result.method(result.name).source_location + "#{self.executable} #{relative_path_for(location)}:#{line}" end def app_root diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 7bcfc86d03..383f485db5 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -988,7 +988,7 @@ module ApplicationTests app 'development' post "/posts.json", '{ "title": "foo", "name": "bar" }', "CONTENT_TYPE" => "application/json" - assert_equal '{"title"=>"foo"}', last_response.body + assert_equal '<ActionController::Parameters {"title"=>"foo"}>', last_response.body end test "config.action_controller.permit_all_parameters = true" do @@ -1444,7 +1444,7 @@ module ApplicationTests assert Rails.configuration.api_only end - test "debug_exception_response_format is :api by default if only_api is enabled" do + test "debug_exception_response_format is :api by default if api_only is enabled" do add_to_config <<-RUBY config.api_only = true RUBY diff --git a/railties/test/application/generators_test.rb b/railties/test/application/generators_test.rb index 84cc6e120b..644af0e737 100644 --- a/railties/test/application/generators_test.rb +++ b/railties/test/application/generators_test.rb @@ -160,5 +160,15 @@ module ApplicationTests assert Rails::Generators.options[:rails][:helper] assert_equal :my_template, Rails::Generators.options[:rails][:template_engine] end + + test "api only generator generate mailer views" do + add_to_config <<-RUBY + config.api_only = true + RUBY + + FileUtils.cd(rails_root){ `bin/rails generate mailer notifier foo` } + assert File.exist?(File.join(rails_root, "app/views/notifier_mailer/foo.text.erb")) + assert File.exist?(File.join(rails_root, "app/views/notifier_mailer/foo.html.erb")) + end end end diff --git a/railties/test/application/integration_test_case_test.rb b/railties/test/application/integration_test_case_test.rb new file mode 100644 index 0000000000..40a79fc636 --- /dev/null +++ b/railties/test/application/integration_test_case_test.rb @@ -0,0 +1,46 @@ +require 'isolation/abstract_unit' + +module ApplicationTests + class IntegrationTestCaseTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + + setup do + build_app + boot_rails + end + + teardown do + teardown_app + end + + test "resets Action Mailer test deliveries" do + script('generate mailer BaseMailer welcome') + + app_file 'test/integration/mailer_integration_test.rb', <<-RUBY + require 'test_helper' + + class MailerIntegrationTest < ActionDispatch::IntegrationTest + setup do + @old_delivery_method = ActionMailer::Base.delivery_method + ActionMailer::Base.delivery_method = :test + end + + teardown do + ActionMailer::Base.delivery_method = @old_delivery_method + end + + 2.times do |i| + define_method "test_resets_deliveries_\#{i}" do + BaseMailer.welcome.deliver_now + assert_equal 1, ActionMailer::Base.deliveries.count + end + end + end + RUBY + + output = Dir.chdir(app_path) { `bin/rails test 2>&1` } + assert_equal 0, $?.to_i, output + assert_match /0 failures, 0 errors/, output + end + end +end diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index 745a3e3ec5..d1c828b509 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -179,12 +179,20 @@ module ApplicationTests app_file "config/routes.rb", <<-RUBY Rails.application.routes.draw do get '/cart', to: 'cart#show' - get '/basketball', to: 'basketball#index' + post '/cart', to: 'cart#create' + get '/basketballs', to: 'basketball#index' end RUBY output = Dir.chdir(app_path){ `bin/rails routes -g show` } assert_equal "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n", output + + output = Dir.chdir(app_path){ `bin/rails routes -g POST` } + assert_equal "Prefix Verb URI Pattern Controller#Action\n POST /cart(.:format) cart#create\n", output + + output = Dir.chdir(app_path){ `bin/rails routes -g basketballs` } + assert_equal " Prefix Verb URI Pattern Controller#Action\n" \ + "basketballs GET /basketballs(.:format) basketball#index\n", output end def test_rake_routes_with_controller_search_key @@ -291,12 +299,11 @@ module ApplicationTests assert_no_match(/Errors running/, output) end - def test_scaffold_with_references_columns_tests_pass_when_belongs_to_is_optional - app_file "config/initializers/active_record_belongs_to_required_by_default.rb", - "Rails.application.config.active_record.belongs_to_required_by_default = false" - + def test_scaffold_with_references_columns_tests_pass_by_default output = Dir.chdir(app_path) do - `bin/rails generate scaffold LineItems product:references cart:belongs_to; + `bin/rails generate model Product; + bin/rails generate model Cart; + bin/rails generate scaffold LineItems product:references cart:belongs_to; RAILS_ENV=test bin/rails db:migrate test` end diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index 7ecadb60ca..b0f348f3f1 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -467,7 +467,7 @@ module ApplicationTests create_test_file :models, 'post', pass: false output = run_test_command('test/models/post_test.rb') - expect = %r{Running:\n\nPostTest\nF\n\nFailure:\nPostTest#test_truth:\nwups!\n\nbin/rails test test/models/post_test.rb:6\n\n\n\n} + expect = %r{Running:\n\nPostTest\nF\n\nFailure:\nPostTest#test_truth \[[^\]]+test/models/post_test.rb:6\]:\nwups!\n\nbin/rails test test/models/post_test.rb:4\n\n\n\n} assert_match expect, output end diff --git a/railties/test/generators/api_app_generator_test.rb b/railties/test/generators/api_app_generator_test.rb index 1ea5661006..8e1cd0891a 100644 --- a/railties/test/generators/api_app_generator_test.rb +++ b/railties/test/generators/api_app_generator_test.rb @@ -73,6 +73,8 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase app/controllers app/mailers app/models + app/views/layouts/mailer.html.erb + app/views/layouts/mailer.text.erb config/environments config/initializers config/locales @@ -94,7 +96,7 @@ class ApiAppGeneratorTest < Rails::Generators::TestCase def skipped_files %w(app/assets app/helpers - app/views + app/views/layouts/application.html.erb config/initializers/assets.rb config/initializers/cookies_serializer.rb config/initializers/session_store.rb diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index f483a0bcbd..921a5b36b5 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -479,6 +479,31 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_inclusion_of_listen_related_configuration_by_default + run_generator + if RbConfig::CONFIG['host_os'] =~ /darwin|linux/ + assert_listen_related_configuration + else + assert_no_listen_related_configuration + end + end + + def test_non_inclusion_of_listen_related_configuration_if_skip_listen + run_generator [destination_root, '--skip-listen'] + assert_no_listen_related_configuration + end + + def test_evented_file_update_checker_config + run_generator + assert_file 'config/environments/development.rb' do |content| + if RbConfig::CONFIG['host_os'] =~ /darwin|linux/ + assert_match(/^\s*config.file_watcher = ActiveSupport::EventedFileUpdateChecker/, content) + else + assert_match(/^\s*# config.file_watcher = ActiveSupport::EventedFileUpdateChecker/, content) + end + end + end + def test_template_from_dir_pwd FileUtils.cd(Rails.root) assert_match(/It works from file!/, run_generator([destination_root, "-m", "lib/template.rb"])) @@ -736,4 +761,23 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "Gemfile", /^\s*gem\s+["']#{gem}["']$*/ end end + + def assert_listen_related_configuration + assert_gem 'listen' + assert_gem 'spring-watcher-listen' + + assert_file 'config/environments/development.rb' do |content| + assert_match(/^\s*config.file_watcher = ActiveSupport::EventedFileUpdateChecker/, content) + end + end + + def assert_no_listen_related_configuration + assert_file 'Gemfile' do |content| + assert_no_match(/listen/, content) + end + + assert_file 'config/environments/development.rb' do |content| + assert_match(/^\s*# config.file_watcher = ActiveSupport::EventedFileUpdateChecker/, content) + end + end end diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index 814f4c050e..c8c8f0aa3b 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -298,18 +298,18 @@ class ModelGeneratorTest < Rails::Generators::TestCase def test_fixtures_use_the_references_ids run_generator ["LineItem", "product:references", "cart:belongs_to"] - assert_file "test/fixtures/line_items.yml", /product: \n cart: / + assert_file "test/fixtures/line_items.yml", /product: one\n cart: one/ assert_generated_fixture("test/fixtures/line_items.yml", - {"one"=>{"product"=>nil, "cart"=>nil}, "two"=>{"product"=>nil, "cart"=>nil}}) + {"one"=>{"product"=>"one", "cart"=>"one"}, "two"=>{"product"=>"two", "cart"=>"two"}}) end def test_fixtures_use_the_references_ids_and_type run_generator ["LineItem", "product:references{polymorphic}", "cart:belongs_to"] - assert_file "test/fixtures/line_items.yml", /product: \n product_type: Product\n cart: / + assert_file "test/fixtures/line_items.yml", /product: one\n product_type: Product\n cart: one/ assert_generated_fixture("test/fixtures/line_items.yml", - {"one"=>{"product"=>nil, "product_type"=>"Product", "cart"=>nil}, - "two"=>{"product"=>nil, "product_type"=>"Product", "cart"=>nil}}) + {"one"=>{"product"=>"one", "product_type"=>"Product", "cart"=>"one"}, + "two"=>{"product"=>"two", "product_type"=>"Product", "cart"=>"two"}}) end def test_fixtures_respect_reserved_yml_keywords diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 6e5132e849..4111a30664 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -335,7 +335,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_file "hyphenated-name/lib/hyphenated/name/engine.rb", /module Hyphenated\n module Name\n class Engine < ::Rails::Engine\n isolate_namespace Hyphenated::Name\n end\n end\nend/ assert_file "hyphenated-name/lib/hyphenated/name.rb", /require "hyphenated\/name\/engine"/ assert_file "hyphenated-name/test/dummy/config/routes.rb", /mount Hyphenated::Name::Engine => "\/hyphenated-name"/ - assert_file "hyphenated-name/app/controllers/hyphenated/name/application_controller.rb", /module Hyphenated\n module Name\n class ApplicationController < ActionController::Base\n protect_from_forgery :with => :exception\n end\n end\nend\n/ + assert_file "hyphenated-name/app/controllers/hyphenated/name/application_controller.rb", /module Hyphenated\n module Name\n class ApplicationController < ActionController::Base\n protect_from_forgery with: :exception\n end\n end\nend\n/ assert_file "hyphenated-name/app/models/hyphenated/name/application_record.rb", /module Hyphenated\n module Name\n class ApplicationRecord < ActiveRecord::Base\n self\.abstract_class = true\n end\n end\nend/ assert_file "hyphenated-name/app/jobs/hyphenated/name/application_job.rb", /module Hyphenated\n module Name\n class ApplicationJob < ActiveJob::Base/ assert_file "hyphenated-name/app/mailers/hyphenated/name/application_mailer.rb", /module Hyphenated\n module Name\n class ApplicationMailer < ActionMailer::Base\n default from: 'from@example.com'\n layout 'mailer'\n end\n end\nend/ @@ -357,7 +357,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_file "my_hyphenated-name/lib/my_hyphenated/name/engine.rb", /module MyHyphenated\n module Name\n class Engine < ::Rails::Engine\n isolate_namespace MyHyphenated::Name\n end\n end\nend/ assert_file "my_hyphenated-name/lib/my_hyphenated/name.rb", /require "my_hyphenated\/name\/engine"/ assert_file "my_hyphenated-name/test/dummy/config/routes.rb", /mount MyHyphenated::Name::Engine => "\/my_hyphenated-name"/ - assert_file "my_hyphenated-name/app/controllers/my_hyphenated/name/application_controller.rb", /module MyHyphenated\n module Name\n class ApplicationController < ActionController::Base\n protect_from_forgery :with => :exception\n end\n end\nend\n/ + assert_file "my_hyphenated-name/app/controllers/my_hyphenated/name/application_controller.rb", /module MyHyphenated\n module Name\n class ApplicationController < ActionController::Base\n protect_from_forgery with: :exception\n end\n end\nend\n/ assert_file "my_hyphenated-name/app/models/my_hyphenated/name/application_record.rb", /module MyHyphenated\n module Name\n class ApplicationRecord < ActiveRecord::Base\n self\.abstract_class = true\n end\n end\nend/ assert_file "my_hyphenated-name/app/jobs/my_hyphenated/name/application_job.rb", /module MyHyphenated\n module Name\n class ApplicationJob < ActiveJob::Base/ assert_file "my_hyphenated-name/app/mailers/my_hyphenated/name/application_mailer.rb", /module MyHyphenated\n module Name\n class ApplicationMailer < ActionMailer::Base\n default from: 'from@example.com'\n layout 'mailer'\n end\n end\nend/ @@ -379,7 +379,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_file "deep-hyphenated-name/lib/deep/hyphenated/name/engine.rb", /module Deep\n module Hyphenated\n module Name\n class Engine < ::Rails::Engine\n isolate_namespace Deep::Hyphenated::Name\n end\n end\n end\nend/ assert_file "deep-hyphenated-name/lib/deep/hyphenated/name.rb", /require "deep\/hyphenated\/name\/engine"/ assert_file "deep-hyphenated-name/test/dummy/config/routes.rb", /mount Deep::Hyphenated::Name::Engine => "\/deep-hyphenated-name"/ - assert_file "deep-hyphenated-name/app/controllers/deep/hyphenated/name/application_controller.rb", /module Deep\n module Hyphenated\n module Name\n class ApplicationController < ActionController::Base\n protect_from_forgery :with => :exception\n end\n end\n end\nend\n/ + assert_file "deep-hyphenated-name/app/controllers/deep/hyphenated/name/application_controller.rb", /module Deep\n module Hyphenated\n module Name\n class ApplicationController < ActionController::Base\n protect_from_forgery with: :exception\n end\n end\n end\nend\n/ assert_file "deep-hyphenated-name/app/models/deep/hyphenated/name/application_record.rb", /module Deep\n module Hyphenated\n module Name\n class ApplicationRecord < ActiveRecord::Base\n self\.abstract_class = true\n end\n end\n end\nend/ assert_file "deep-hyphenated-name/app/jobs/deep/hyphenated/name/application_job.rb", /module Deep\n module Hyphenated\n module Name\n class ApplicationJob < ActiveJob::Base/ assert_file "deep-hyphenated-name/app/mailers/deep/hyphenated/name/application_mailer.rb", /module Deep\n module Hyphenated\n module Name\n class ApplicationMailer < ActionMailer::Base\n default from: 'from@example.com'\n layout 'mailer'\n end\n end\n end\nend/ diff --git a/railties/test/generators/plugin_test_runner_test.rb b/railties/test/generators/plugin_test_runner_test.rb index f492cd49ef..ef6359fece 100644 --- a/railties/test/generators/plugin_test_runner_test.rb +++ b/railties/test/generators/plugin_test_runner_test.rb @@ -63,7 +63,7 @@ class PluginTestRunnerTest < ActiveSupport::TestCase create_test_file 'post', pass: false output = run_test_command('test/post_test.rb') - expect = %r{Running:\n\nPostTest\nF\n\nFailure:\nPostTest#test_truth:\nwups!\n\nbin/test (/private)?#{plugin_path}/test/post_test.rb:6} + expect = %r{Running:\n\nPostTest\nF\n\nFailure:\nPostTest#test_truth \[[^\]]+test/post_test.rb:6\]:\nwups!\n\nbin/test (/private)?#{plugin_path}/test/post_test.rb:4} assert_match expect, output end diff --git a/railties/test/generators/test_runner_in_engine_test.rb b/railties/test/generators/test_runner_in_engine_test.rb index 69906b962b..d37e261fbb 100644 --- a/railties/test/generators/test_runner_in_engine_test.rb +++ b/railties/test/generators/test_runner_in_engine_test.rb @@ -17,7 +17,7 @@ class TestRunnerInEngineTest < ActiveSupport::TestCase create_test_file 'post', pass: false output = run_test_command('test/post_test.rb') - expect = %r{Running:\n\nPostTest\nF\n\nFailure:\nPostTest#test_truth:\nwups!\n\nbin/rails test test/post_test.rb:6} + expect = %r{Running:\n\nPostTest\nF\n\nFailure:\nPostTest#test_truth \[[^\]]+test/post_test.rb:6\]:\nwups!\n\nbin/rails test test/post_test.rb:4} assert_match expect, output end diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index dddf8bd257..66a8cf54af 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -272,10 +272,17 @@ module TestHelpers end def remove_from_config(str) - file = "#{app_path}/config/application.rb" + remove_from_file("#{app_path}/config/application.rb", str) + end + + def remove_from_env_config(env, str) + remove_from_file("#{app_path}/config/environments/#{env}.rb", str) + end + + def remove_from_file(file, str) contents = File.read(file) - contents.sub!(/#{str}/, "") - File.open(file, "w+") { |f| f.puts contents } + contents.sub!(/#{str}/, '') + File.write(file, contents) end def app_file(path, contents, mode = 'w') @@ -319,7 +326,6 @@ class ActiveSupport::TestCase include ActiveSupport::Testing::Stream self.test_order = :sorted - end # Create a scope and build a fixture rails app @@ -333,7 +339,7 @@ Module.new do environment = File.expand_path('../../../../load_paths', __FILE__) require_environment = "-r #{environment}" - `#{Gem.ruby} #{require_environment} #{RAILS_FRAMEWORK_ROOT}/railties/exe/rails new #{app_template_path} --skip-gemfile --no-rc` + `#{Gem.ruby} #{require_environment} #{RAILS_FRAMEWORK_ROOT}/railties/exe/rails new #{app_template_path} --skip-gemfile --skip-listen --no-rc` File.open("#{app_template_path}/config/boot.rb", 'w') do |f| f.puts "require '#{environment}'" f.puts "require 'rails/all'" diff --git a/railties/test/test_unit/reporter_test.rb b/railties/test/test_unit/reporter_test.rb index 2d08d4ec30..0d64b48550 100644 --- a/railties/test/test_unit/reporter_test.rb +++ b/railties/test/test_unit/reporter_test.rb @@ -62,7 +62,7 @@ class TestUnitReporterTest < ActiveSupport::TestCase @reporter.record(failed_test) @reporter.report - expect = %r{\AF\n\nFailure:\nTestUnitReporterTest::ExampleTest#woot:\nboo\n\nbin/rails test test/test_unit/reporter_test.rb:\d+\n\n\z} + expect = %r{\AF\n\nFailure:\nTestUnitReporterTest::ExampleTest#woot \[[^\]]+\]:\nboo\n\nbin/rails test test/test_unit/reporter_test.rb:\d+\n\n\z} assert_match expect, @output.string end @@ -79,7 +79,7 @@ class TestUnitReporterTest < ActiveSupport::TestCase verbose.record(skipped_test) verbose.report - expect = %r{\ATestUnitReporterTest::ExampleTest#woot = 10\.00 s = S\n\n\nSkipped:\nTestUnitReporterTest::ExampleTest#woot:\nskipchurches, misstemples\n\nbin/rails test test/test_unit/reporter_test.rb:\d+\n\n\z} + expect = %r{\ATestUnitReporterTest::ExampleTest#woot = 10\.00 s = S\n\n\nSkipped:\nTestUnitReporterTest::ExampleTest#woot \[[^\]]+\]:\nskipchurches, misstemples\n\nbin/rails test test/test_unit/reporter_test.rb:\d+\n\n\z} assert_match expect, @output.string end @@ -104,11 +104,22 @@ class TestUnitReporterTest < ActiveSupport::TestCase end end - test "fail fast does not interrupt run errors or skips" do + test "fail fast interrupts run on error" do fail_fast = Rails::TestUnitReporter.new @output, fail_fast: true + interrupt_raised = false - fail_fast.record(errored_test) - assert_no_match 'Failed tests:', @output.string + # Minitest passes through Interrupt, catch it manually. + begin + fail_fast.record(errored_test) + rescue Interrupt + interrupt_raised = true + ensure + assert interrupt_raised, 'Expected Interrupt to be raised.' + end + end + + test "fail fast does not interrupt run skips" do + fail_fast = Rails::TestUnitReporter.new @output, fail_fast: true fail_fast.record(skipped_test) assert_no_match 'Failed tests:', @output.string diff --git a/tools/README.md b/tools/README.md index 25ab798bd5..1f3d6c59d9 100644 --- a/tools/README.md +++ b/tools/README.md @@ -5,3 +5,4 @@ They aren't used by Rails apps directly. * `console` drops you in irb and loads local Rails repos * `profile` profiles `Kernel#require` to help reduce startup time + * `line_statistics` provides CodeTools module and LineStatistics class to count lines
\ No newline at end of file |