diff options
162 files changed, 1489 insertions, 836 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index d12aa0772f..892a9a40f0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -28,9 +28,9 @@ GIT GIT remote: git://github.com/turbolinks/turbolinks-rails.git - revision: 1604dcd7bad911f1471d65e4f47cd19d844354f1 + revision: 65884729016dbb4d032f12bb01b7e7c1ddeb68ac specs: - turbolinks (5.0.0.beta1) + turbolinks (5.0.0.beta2) turbolinks-source PATH @@ -252,7 +252,7 @@ GEM thor (0.19.1) thread (0.1.7) thread_safe (0.3.5) - turbolinks-source (5.0.0.beta1.1) + turbolinks-source (5.0.0.beta2) tzinfo (1.2.2) thread_safe (~> 0.1) tzinfo-data (1.2015.7) diff --git a/actioncable/README.md b/actioncable/README.md index c85d59a1c8..334c75c79c 100644 --- a/actioncable/README.md +++ b/actioncable/README.md @@ -39,7 +39,7 @@ reflections of each unit. ### A full-stack example The first thing you must do is define your `ApplicationCable::Connection` class in Ruby. This -is the place where you authorize the incoming connection, and proceed to establish it +is the place where you authorize the incoming connection, and proceed to establish it, if all is well. Here's the simplest example starting with the server-side connection class: ```ruby @@ -73,7 +73,7 @@ use that to set the `current_user`. By identifying the connection by this same c you're also ensuring that you can later retrieve all open connections by a given user (and potentially disconnect them all if the user is deleted or deauthorized). -Then you should define your `ApplicationCable::Channel` class in Ruby. This is the place where you put +Next, you should define your `ApplicationCable::Channel` class in Ruby. This is the place where you put shared logic between your channels. ```ruby @@ -94,7 +94,7 @@ The client-side needs to setup a consumer instance of this connection. That's do App.cable = ActionCable.createConsumer("ws://cable.example.com") ``` -The ws://cable.example.com address must point to your set of Action Cable servers, and it +The `ws://cable.example.com` address must point to your Action Cable server(s), and it must share a cookie namespace with the rest of the application (which may live under http://example.com). This ensures that the signed cookie will be correctly sent. @@ -105,8 +105,8 @@ is defined by declaring channels on the server and allowing the consumer to subs ### Channel example 1: User appearances -Here's a simple example of a channel that tracks whether a user is online or not and what page they're on. -(This is useful for creating presence features like showing a green dot next to a user name if they're online). +Here's a simple example of a channel that tracks whether a user is online or not, and also what page they are currently on. +(This is useful for creating presence features like showing a green dot next to a user's name if they're online). First you declare the server-side channel: @@ -180,7 +180,7 @@ App.cable.subscriptions.create "AppearanceChannel", Simply calling `App.cable.subscriptions.create` will setup the subscription, which will call `AppearanceChannel#subscribed`, which in turn is linked to original `App.cable` -> `ApplicationCable::Connection` instances. -We then link the client-side `appear` method to `AppearanceChannel#appear(data)`. This is possible because the server-side +Next, we link the client-side `appear` method to `AppearanceChannel#appear(data)`. This is possible because the server-side channel instance will automatically expose the public methods declared on the class (minus the callbacks), so that these can be reached as remote procedure calls via a subscription's `perform` method. @@ -215,7 +215,7 @@ ActionCable.server.broadcast \ "web_notifications_#{current_user.id}", { title: 'New things!', body: 'All the news that is fit to print' } ``` -The `ActionCable.server.broadcast` call places a message in the Redis' pubsub queue under a separate broadcasting name for each user. For a user with an ID of 1, the broadcasting name would be `web_notifications_1`. +The `ActionCable.server.broadcast` call places a message in the Action Cable pubsub queue under a separate broadcasting name for each user. For a user with an ID of 1, the broadcasting name would be `web_notifications_1`. The channel has been instructed to stream everything that arrives at `web_notifications_1` directly to the client by invoking the `#received(data)` callback. The data is the hash sent as the second parameter to the server-side broadcast call, JSON encoded for the trip across the wire, and unpacked for the data argument arriving to `#received`. @@ -234,7 +234,7 @@ class ChatChannel < ApplicationCable::Channel end ``` -Pass an object as the first argument to `subscriptions.create`, and that object will become your params hash in your cable channel. The keyword `channel` is required. +If you pass an object as the first argument to `subscriptions.create`, that object will become the params hash in your cable channel. The keyword `channel` is required. ```coffeescript # Client-side, which assumes you've already requested the right to send web notifications @@ -293,7 +293,7 @@ The rebroadcast will be received by all connected clients, _including_ the clien ### More complete examples -See the [rails/actioncable-examples](http://github.com/rails/actioncable-examples) repository for a full example of how to setup Action Cable in a Rails app and adding channels. +See the [rails/actioncable-examples](http://github.com/rails/actioncable-examples) repository for a full example of how to setup Action Cable in a Rails app, and how to add channels. ## Configuration @@ -349,11 +349,11 @@ something like: `App.cable = ActionCable.createConsumer("/cable")`. The second option is to pass the server url through the `action_cable_meta_tag` in your layout. This uses a url or path typically set via `config.action_cable.url` in the environment configuration files, or defaults to "/cable". -This method is especially useful if your websocket url might change between environments. If you host your production server via https, you will need to use the wss scheme -for your ActionCable server, but development might remain http and use the ws scheme. You might use localhost in development and your +This method is especially useful if your WebSocket url might change between environments. If you host your production server via https, you will need to use the wss scheme +for your Action Cable server, but development might remain http and use the ws scheme. You might use localhost in development and your domain in production. -In any case, to vary the websocket url between environments, add the following configuration to each environment: +In any case, to vary the WebSocket url between environments, add the following configuration to each environment: ```ruby config.action_cable.url = "ws://example.com:28080" @@ -412,7 +412,7 @@ The above will start a cable server on port 28080. ### In app -If you are using a threaded server like Puma or Thin, the current implementation of ActionCable can run side-along with your Rails application. For example, to listen for WebSocket requests on `/cable`, mount the server at that path: +If you are using a threaded server like Puma or Thin, the current implementation of Action Cable can run side-along with your Rails application. For example, to listen for WebSocket requests on `/cable`, mount the server at that path: ```ruby # config/routes.rb @@ -421,7 +421,7 @@ Example::Application.routes.draw do end ``` -For every instance of your server you create and for every worker your server spawns, you will also have a new instance of ActionCable, but the use of Redis keeps messages synced across connections. +For every instance of your server you create and for every worker your server spawns, you will also have a new instance of Action Cable, but the use of Redis keeps messages synced across connections. ### Notes @@ -433,14 +433,14 @@ The WebSocket server doesn't have access to the session, but it has access to th ## Dependencies -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. +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 Action Cable 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). ## Deployment -Action Cable is powered by a combination of websockets and threads. All of the +Action Cable is powered by a combination of WebSockets and threads. All of the connection management is handled internally by utilizing Ruby’s native thread support, which means you can use all your regular Rails models with no problems as long as you haven’t committed any thread-safety sins. diff --git a/actioncable/app/assets/javascripts/action_cable.coffee.erb b/actioncable/app/assets/javascripts/action_cable.coffee.erb index 18a48c0610..d95fe78ac5 100644 --- a/actioncable/app/assets/javascripts/action_cable.coffee.erb +++ b/actioncable/app/assets/javascripts/action_cable.coffee.erb @@ -21,3 +21,14 @@ a.href else url + + startDebugging: -> + @debugging = true + + stopDebugging: -> + @debugging = null + + log: (messages...) -> + if @debugging + messages.push(Date.now()) + console.log("[ActionCable]", messages...) diff --git a/actioncable/app/assets/javascripts/action_cable/connection.coffee b/actioncable/app/assets/javascripts/action_cable/connection.coffee index fbd7dbd35b..ee888f567b 100644 --- a/actioncable/app/assets/javascripts/action_cable/connection.coffee +++ b/actioncable/app/assets/javascripts/action_cable/connection.coffee @@ -16,9 +16,12 @@ class ActionCable.Connection false open: => - if @webSocket and not @isState("closed") + if @isAlive() + ActionCable.log("Attemped to open WebSocket, but existing socket is #{@getState()}") throw new Error("Existing connection must be closed before opening") else + ActionCable.log("Opening WebSocket, current state is #{@getState()}") + @uninstallEventHandlers() if @webSocket? @webSocket = new WebSocket(@consumer.url) @installEventHandlers() true @@ -27,19 +30,26 @@ class ActionCable.Connection @webSocket?.close() reopen: -> - if @isState("closed") - @open() - else + ActionCable.log("Reopening WebSocket, current state is #{@getState()}") + if @isAlive() try @close() + catch error + ActionCable.log("Failed to reopen WebSocket", error) finally + ActionCable.log("Reopening WebSocket in #{@constructor.reopenDelay}ms") setTimeout(@open, @constructor.reopenDelay) + else + @open() isOpen: -> @isState("open") # Private + isAlive: -> + @webSocket? and not @isState("closing", "closed") + isState: (states...) -> @getState() in states @@ -53,6 +63,11 @@ class ActionCable.Connection @webSocket["on#{eventName}"] = handler return + uninstallEventHandlers: -> + for eventName of @events + @webSocket["on#{eventName}"] = -> + return + events: message: (event) -> {identifier, message, type} = JSON.parse(event.data) @@ -66,13 +81,16 @@ class ActionCable.Connection @consumer.subscriptions.notify(identifier, "received", message) open: -> + ActionCable.log("WebSocket onopen event") @disconnected = false @consumer.subscriptions.reload() close: -> + ActionCable.log("WebSocket onclose event") @disconnect() error: -> + ActionCable.log("WebSocket onerror event") @disconnect() disconnect: -> diff --git a/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee b/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee index 99b9a1c6d5..75a6f1fb07 100644 --- a/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee +++ b/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee @@ -17,6 +17,7 @@ class ActionCable.ConnectionMonitor @reset() @pingedAt = now() delete @disconnectedAt + ActionCable.log("ConnectionMonitor connected") disconnected: -> @disconnectedAt = now() @@ -33,10 +34,12 @@ class ActionCable.ConnectionMonitor @startedAt = now() @poll() document.addEventListener("visibilitychange", @visibilityDidChange) + ActionCable.log("ConnectionMonitor started, pollInterval is #{@getInterval()}ms") stop: -> @stoppedAt = now() document.removeEventListener("visibilitychange", @visibilityDidChange) + ActionCable.log("ConnectionMonitor stopped") poll: -> setTimeout => @@ -52,8 +55,12 @@ class ActionCable.ConnectionMonitor reconnectIfStale: -> if @connectionIsStale() + ActionCable.log("ConnectionMonitor detected stale connection, reconnectAttempts = #{@reconnectAttempts}") @reconnectAttempts++ - unless @disconnectedRecently() + if @disconnectedRecently() + ActionCable.log("ConnectionMonitor skipping reopen because recently disconnected at #{@disconnectedAt}") + else + ActionCable.log("ConnectionMonitor reopening") @consumer.connection.reopen() connectionIsStale: -> @@ -66,6 +73,7 @@ class ActionCable.ConnectionMonitor if document.visibilityState is "visible" setTimeout => if @connectionIsStale() or not @consumer.connection.isOpen() + ActionCable.log("ConnectionMonitor reopening stale connection after visibilitychange to #{document.visibilityState}") @consumer.connection.reopen() , 200 diff --git a/actioncable/lib/action_cable/channel/base.rb b/actioncable/lib/action_cable/channel/base.rb index 86c8f12f6e..05764fe107 100644 --- a/actioncable/lib/action_cable/channel/base.rb +++ b/actioncable/lib/action_cable/channel/base.rb @@ -32,8 +32,8 @@ module ActionCable # # == Action processing # - # Unlike subclasses of ActionController::Base, channels do not follow a REST - # constraint form for their actions. Instead, ActionCable operates through a + # Unlike subclasses of ActionController::Base, channels do not follow a RESTful + # constraint form for their actions. Instead, Action Cable operates through a # remote-procedure call model. You can declare any public method on the # channel (optionally taking a <tt>data</tt> argument), and this method is # automatically exposed as callable to the client. @@ -63,10 +63,10 @@ module ActionCable # end # end # - # In this example, subscribed/unsubscribed are not callable methods, as they + # In this example, the subscribed and unsubscribed methods are not callable methods, as they # were already declared in ActionCable::Channel::Base, but <tt>#appear</tt> # and <tt>#away</tt> are. <tt>#generate_connection_token</tt> is also not - # callable as it's a private method. You'll see that appear accepts a data + # callable, since it's a private method. You'll see that appear accepts a data # parameter, which it then uses as part of its model call. <tt>#away</tt> # does not, since it's simply a trigger action. # @@ -125,7 +125,7 @@ module ActionCable protected # action_methods are cached and there is sometimes need to refresh # them. ::clear_action_methods! allows you to do that, so next time - # you run action_methods, they will be recalculated + # you run action_methods, they will be recalculated. def clear_action_methods! @action_methods = nil end @@ -166,9 +166,9 @@ module ActionCable end end - # Called by the cable connection when its cut so the channel has a chance to cleanup with callbacks. + # Called by the cable connection when its cut, so the channel has a chance to cleanup with callbacks. # This method is not intended to be called directly by the user. Instead, overwrite the #unsubscribed callback. - def unsubscribe_from_channel + def unsubscribe_from_channel # :nodoc: run_callbacks :unsubscribe do unsubscribed end @@ -183,7 +183,7 @@ module ActionCable end # Called once a consumer has cut its cable connection. Can be used for cleaning up connections or marking - # people as offline or the like. + # users as offline or the like. def unsubscribed # Override in subclasses end @@ -224,7 +224,6 @@ module ActionCable end end - def subscribe_to_channel run_callbacks :subscribe do subscribed @@ -237,7 +236,6 @@ module ActionCable end end - def extract_action(data) (data['action'].presence || :receive).to_sym end diff --git a/actioncable/lib/action_cable/channel/periodic_timers.rb b/actioncable/lib/action_cable/channel/periodic_timers.rb index 56597d02d7..0f6e854520 100644 --- a/actioncable/lib/action_cable/channel/periodic_timers.rb +++ b/actioncable/lib/action_cable/channel/periodic_timers.rb @@ -12,7 +12,7 @@ module ActionCable end module ClassMethods - # Allow you to call a private method <tt>every</tt> so often seconds. This periodic timer can be useful + # Allows you to call a private method <tt>every</tt> so often seconds. This periodic timer can be useful # for sending a steady flow of updates to a client based off an object that was configured on subscription. # It's an alternative to using streams if the channel is able to do the work internally. def periodically(callback, every:) diff --git a/actioncable/lib/action_cable/channel/streams.rb b/actioncable/lib/action_cable/channel/streams.rb index 3158f30814..3e3be4cd44 100644 --- a/actioncable/lib/action_cable/channel/streams.rb +++ b/actioncable/lib/action_cable/channel/streams.rb @@ -1,8 +1,8 @@ module ActionCable module Channel # Streams allow channels to route broadcastings to the subscriber. A broadcasting is, as discussed elsewhere, a pub/sub queue where any data - # put into it is automatically sent to the clients that are connected at that time. It's purely an online queue, though. If you're not - # streaming a broadcasting at the very moment it sends out an update, you'll not get that update when connecting later. + # placed into it is automatically sent to the clients that are connected at that time. It's purely an online queue, though. If you're not + # streaming a broadcasting at the very moment it sends out an update, you will not get that update, if you connect after it has been sent. # # Most commonly, the streamed broadcast is sent straight to the subscriber on the client-side. The channel just acts as a connector between # the two parties (the broadcaster and the channel subscriber). Here's an example of a channel that allows subscribers to get all new @@ -18,8 +18,10 @@ module ActionCable # end # end # - # So the subscribers of this channel will get whatever data is put into the, let's say, `comments_for_45` broadcasting as soon as it's put there. - # That looks like so from that side of things: + # Based on the above example, the subscribers of this channel will get whatever data is put into the, + # let's say, `comments_for_45` broadcasting as soon as it's put there. + # + # An example broadcasting for this channel looks like so: # # ActionCable.server.broadcast "comments_for_45", author: 'DHH', content: 'Rails is just swell' # @@ -37,8 +39,8 @@ module ActionCable # # CommentsChannel.broadcast_to(@post, @comment) # - # If you don't just want to parlay the broadcast unfiltered to the subscriber, you can supply a callback that lets you alter what goes out. - # Example below shows how you can use this to provide performance introspection in the process: + # If you don't just want to parlay the broadcast unfiltered to the subscriber, you can also supply a callback that lets you alter what is sent out. + # The below example shows how you can use this to provide performance introspection in the process: # # class ChatChannel < ApplicationCable::Channel # def subscribed @@ -70,7 +72,7 @@ module ActionCable # Start streaming from the named <tt>broadcasting</tt> pubsub queue. Optionally, you can pass a <tt>callback</tt> that'll be used # instead of the default of just transmitting the updates straight to the subscriber. def stream_from(broadcasting, callback = nil) - # Hold off the confirmation until pubsub#subscribe is successful + # Don't send the confirmation until pubsub#subscribe is successful defer_subscription_confirmation! callback ||= default_stream_callback(broadcasting) diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb index 1acef93025..60f3ad3e06 100644 --- a/actioncable/lib/action_cable/connection/base.rb +++ b/actioncable/lib/action_cable/connection/base.rb @@ -2,9 +2,9 @@ require 'action_dispatch' module ActionCable module Connection - # For every WebSocket the cable server is accepting, a Connection object will be instantiated. This instance becomes the parent - # of all the channel subscriptions that are created from there on. Incoming messages are then routed to these channel subscriptions - # based on an identifier sent by the cable consumer. The Connection itself does not deal with any specific application logic beyond + # For every WebSocket the Action Cable server accepts, a Connection object will be instantiated. This instance becomes the parent + # of all of the channel subscriptions that are created from there on. Incoming messages are then routed to these channel subscriptions + # based on an identifier sent by the Action Cable consumer. The Connection itself does not deal with any specific application logic beyond # authentication and authorization. # # Here's a basic example: @@ -33,8 +33,8 @@ module ActionCable # end # end # - # 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 + # First, we declare that this connection can be identified by its current_user. This allows us to later be able to find all connections + # established for that current_user (and potentially disconnect them). You can declare as many # 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 @@ -65,8 +65,8 @@ module ActionCable end # Called by the server when a new WebSocket connection is established. This configures the callbacks intended for overwriting by the user. - # This method should not be called directly. Rely on the #connect (and #disconnect) callback instead. - def process + # This method should not be called directly -- instead rely upon on the #connect (and #disconnect) callbacks. + def process # :nodoc: logger.info started_request_message if websocket.possible? && allow_request_origin? @@ -76,7 +76,7 @@ module ActionCable end end - # Data received over the cable is handled by this method. It's expected that everything inbound is JSON encoded. + # Data received over the WebSocket connection is handled by this method. It's expected that everything inbound is JSON encoded. # The data is routed to the proper channel that the connection has subscribed to. def receive(data_in_json) if websocket.alive? @@ -88,7 +88,7 @@ module ActionCable # Send raw data straight back down the WebSocket. This is not intended to be called directly. Use the #transmit available on the # Channel instead, as that'll automatically address the correct subscriber and wrap the message in JSON. - def transmit(data) + def transmit(data) # :nodoc: websocket.transmit data end @@ -154,7 +154,7 @@ module ActionCable def handle_open connect if respond_to?(:connect) subscribe_to_internal_channel - beat + confirm_connection_monitor_subscription message_buffer.process! server.add_connection(self) @@ -173,6 +173,13 @@ module ActionCable disconnect if respond_to?(:disconnect) end + def confirm_connection_monitor_subscription + # Send confirmation message to the internal connection monitor channel. + # This ensures the connection monitor state is reset after a successful + # websocket connection. + transmit ActiveSupport::JSON.encode(identifier: ActionCable::INTERNAL[:identifiers][:ping], type: ActionCable::INTERNAL[:message_types][:confirmation]) + end + def allow_request_origin? return true if server.config.disable_request_forgery_protection diff --git a/actioncable/lib/action_cable/connection/identification.rb b/actioncable/lib/action_cable/connection/identification.rb index 885ff3f102..4a54044aff 100644 --- a/actioncable/lib/action_cable/connection/identification.rb +++ b/actioncable/lib/action_cable/connection/identification.rb @@ -12,7 +12,7 @@ module ActionCable class_methods do # Mark a key as being a connection identifier index that can then be used to find the specific connection again later. - # Common identifiers are current_user and current_account, but could be anything really. + # Common identifiers are current_user and current_account, but could be anything, really. # # Note that anything marked as an identifier will automatically create a delegate by the same name on any # channel instances created off the connection. diff --git a/actioncable/lib/action_cable/connection/message_buffer.rb b/actioncable/lib/action_cable/connection/message_buffer.rb index 2f65a1e84a..19f2e6e918 100644 --- a/actioncable/lib/action_cable/connection/message_buffer.rb +++ b/actioncable/lib/action_cable/connection/message_buffer.rb @@ -1,8 +1,7 @@ module ActionCable module Connection - # Allows us to buffer messages received from the WebSocket before the Connection has been fully initialized and is ready to receive them. - # Entirely internal operation and should not be used directly by the user. - class MessageBuffer + # Allows us to buffer messages received from the WebSocket before the Connection has been fully initialized, and is ready to receive them. + class MessageBuffer # :nodoc: def initialize(connection) @connection = connection @buffered_messages = [] diff --git a/actioncable/lib/action_cable/connection/subscriptions.rb b/actioncable/lib/action_cable/connection/subscriptions.rb index 24934e12f2..3742f248d1 100644 --- a/actioncable/lib/action_cable/connection/subscriptions.rb +++ b/actioncable/lib/action_cable/connection/subscriptions.rb @@ -3,8 +3,8 @@ require 'active_support/core_ext/hash/indifferent_access' module ActionCable module Connection # Collection class for all the channel subscriptions established on a given connection. Responsible for routing incoming commands that arrive on - # the connection to the proper channel. Should not be used directly by the user. - class Subscriptions + # the connection to the proper channel. + class Subscriptions # :nodoc: def initialize(connection) @connection = connection @subscriptions = {} diff --git a/actioncable/lib/action_cable/helpers/action_cable_helper.rb b/actioncable/lib/action_cable/helpers/action_cable_helper.rb index b82751468a..3067542b33 100644 --- a/actioncable/lib/action_cable/helpers/action_cable_helper.rb +++ b/actioncable/lib/action_cable/helpers/action_cable_helper.rb @@ -9,7 +9,7 @@ module ActionCable # <%= javascript_include_tag 'application', 'data-turbolinks-track' => true %> # </head> # - # This is then used by ActionCable to determine the url of your websocket server. + # This is then used by Action Cable to determine the url of your WebSocket server. # Your CoffeeScript can then connect to the server without needing to specify the # url directly: # diff --git a/actioncable/lib/action_cable/remote_connections.rb b/actioncable/lib/action_cable/remote_connections.rb index 7ec121308a..aeef8abc72 100644 --- a/actioncable/lib/action_cable/remote_connections.rb +++ b/actioncable/lib/action_cable/remote_connections.rb @@ -13,8 +13,8 @@ module ActionCable # ActionCable.server.remote_connections.where(current_user: User.find(1)).disconnect # # This will disconnect all the connections established for - # <tt>User.find(1)</tt> across all servers running on all machines, because - # it uses the internal channel that all these servers are subscribed to. + # <tt>User.find(1)</tt>, across all servers running on all machines, because + # it uses the internal channel that all of these servers are subscribed to. class RemoteConnections attr_reader :server diff --git a/actioncable/lib/action_cable/server/base.rb b/actioncable/lib/action_cable/server/base.rb index fe48c112df..c3b64299e3 100644 --- a/actioncable/lib/action_cable/server/base.rb +++ b/actioncable/lib/action_cable/server/base.rb @@ -2,10 +2,10 @@ require 'thread' module ActionCable module Server - # A singleton ActionCable::Server instance is available via ActionCable.server. It's used by the rack process that starts the cable server, but - # also by the user to reach the RemoteConnections instead for finding and disconnecting connections across all servers. + # A singleton ActionCable::Server instance is available via ActionCable.server. It's used by the Rack process that starts the Action Cable server, but + # is also used by the user to reach the RemoteConnections object, which is used for finding and disconnecting connections across all servers. # - # Also, this is the server instance used for broadcasting. See Broadcasting for details. + # Also, this is the server instance used for broadcasting. See Broadcasting for more information. class Base include ActionCable::Server::Broadcasting include ActionCable::Server::Connections @@ -19,11 +19,10 @@ module ActionCable def initialize @mutex = Mutex.new - @remote_connections = @stream_event_loop = @worker_pool = @channel_classes = @pubsub = nil end - # Called by rack to setup the server. + # Called by Rack to setup the server. def call(env) setup_heartbeat_timer config.connection_class.new(self, env).process @@ -48,7 +47,7 @@ module ActionCable @worker_pool || @mutex.synchronize { @worker_pool ||= ActionCable::Server::Worker.new(max_size: config.worker_pool_size) } end - # Requires and returns a hash of all the channel class constants keyed by name. + # Requires and returns a hash of all of the channel class constants, which are keyed by name. def channel_classes @channel_classes || @mutex.synchronize do @channel_classes ||= begin @@ -63,7 +62,7 @@ module ActionCable @pubsub || @mutex.synchronize { @pubsub ||= config.pubsub_adapter.new(self) } end - # All the identifiers applied to the connection class associated with this server. + # All of the identifiers applied to the connection class associated with this server. def connection_identifiers config.connection_class.identifiers end diff --git a/actioncable/lib/action_cable/server/broadcasting.rb b/actioncable/lib/action_cable/server/broadcasting.rb index b87232671b..f90fe7b9e2 100644 --- a/actioncable/lib/action_cable/server/broadcasting.rb +++ b/actioncable/lib/action_cable/server/broadcasting.rb @@ -1,6 +1,6 @@ module ActionCable module Server - # Broadcasting is how other parts of your application can send messages to the channel subscribers. As explained in Channel, most of the time, these + # Broadcasting is how other parts of your application can send messages to a channel's subscribers. As explained in Channel, most of the time, these # broadcastings are streamed directly to the clients subscribed to the named broadcasting. Let's explain with a full-stack example: # # class WebNotificationsChannel < ApplicationCable::Channel @@ -9,16 +9,16 @@ module ActionCable # end # end # - # # Somewhere in your app this is called, perhaps from a NewCommentJob + # # Somewhere in your app this is called, perhaps from a NewCommentJob: # ActionCable.server.broadcast \ # "web_notifications_1", { title: "New things!", body: "All that's fit for print" } # - # # Client-side CoffeeScript, which assumes you've already requested the right to send web notifications + # # Client-side CoffeeScript, which assumes you've already requested the right to send web notifications: # App.cable.subscriptions.create "WebNotificationsChannel", # received: (data) -> # new Notification data['title'], body: data['body'] module Broadcasting - # Broadcast a hash directly to a named <tt>broadcasting</tt>. It'll automatically be JSON encoded. + # Broadcast a hash directly to a named <tt>broadcasting</tt>. This will later be JSON encoded. def broadcast(broadcasting, message) broadcaster_for(broadcasting).broadcast(message) end diff --git a/actioncable/lib/action_cable/server/configuration.rb b/actioncable/lib/action_cable/server/configuration.rb index 58bb8ff65a..ee17bff13b 100644 --- a/actioncable/lib/action_cable/server/configuration.rb +++ b/actioncable/lib/action_cable/server/configuration.rb @@ -1,6 +1,6 @@ module ActionCable module Server - # An instance of this configuration object is available via ActionCable.server.config, which allows you to tweak the configuration points + # An instance of this configuration object is available via ActionCable.server.config, which allows you to tweak Action Cable configuration # in a Rails config initializer. class Configuration attr_accessor :logger, :log_tags diff --git a/actioncable/lib/action_cable/server/connections.rb b/actioncable/lib/action_cable/server/connections.rb index 8671dd5ebd..4dc8934b25 100644 --- a/actioncable/lib/action_cable/server/connections.rb +++ b/actioncable/lib/action_cable/server/connections.rb @@ -1,9 +1,8 @@ module ActionCable module Server - # Collection class for all the connections that's been established on this specific server. Remember, usually you'll run many cable servers, so - # you can't use this collection as an full list of all the connections established against your application. Use RemoteConnections for that. - # As such, this is primarily for internal use. - module Connections + # Collection class for all the connections that have been established on this specific server. Remember, usually you'll run many Action Cable servers, so + # you can't use this collection as a full list of all of the connections established against your application. Instead, use RemoteConnections for that. + module Connections # :nodoc: BEAT_INTERVAL = 3 def connections @@ -19,7 +18,7 @@ module ActionCable end # WebSocket connection implementations differ on when they'll mark a connection as stale. We basically never want a connection to go stale, as you - # then can't rely on being able to receive and send to it. So there's a 3 second heartbeat running on all connections. If the beat fails, we automatically + # then can't rely on being able to communicate with the connection. To solve this, a 3 second heartbeat runs on all connections. If the beat fails, we automatically # disconnect. def setup_heartbeat_timer @heartbeat_timer ||= Concurrent::TimerTask.new(execution_interval: BEAT_INTERVAL) do diff --git a/actioncable/lib/action_cable/server/worker.rb b/actioncable/lib/action_cable/server/worker.rb index 3b6c6d44a1..b920b880db 100644 --- a/actioncable/lib/action_cable/server/worker.rb +++ b/actioncable/lib/action_cable/server/worker.rb @@ -4,8 +4,8 @@ require 'concurrent' module ActionCable module Server - # Worker used by Server.send_async to do connection work in threads. Only for internal use. - class Worker + # Worker used by Server.send_async to do connection work in threads. + class Worker # :nodoc: include ActiveSupport::Callbacks thread_mattr_accessor :connection diff --git a/actioncable/lib/action_cable/server/worker/active_record_connection_management.rb b/actioncable/lib/action_cable/server/worker/active_record_connection_management.rb index ecece4e270..1ac8934410 100644 --- a/actioncable/lib/action_cable/server/worker/active_record_connection_management.rb +++ b/actioncable/lib/action_cable/server/worker/active_record_connection_management.rb @@ -1,7 +1,7 @@ module ActionCable module Server class Worker - # Clear active connections between units of work so the long-running channel or connection processes do not hoard connections. + # Clear active connections between units of work so that way long-running channels or connection processes do not hoard connections. module ActiveRecordConnectionManagement extend ActiveSupport::Concern @@ -19,4 +19,4 @@ module ActionCable end end end -end
\ No newline at end of file +end diff --git a/actioncable/lib/action_cable/subscription_adapter/evented_redis.rb b/actioncable/lib/action_cable/subscription_adapter/evented_redis.rb index af04a58c70..e6c862959b 100644 --- a/actioncable/lib/action_cable/subscription_adapter/evented_redis.rb +++ b/actioncable/lib/action_cable/subscription_adapter/evented_redis.rb @@ -13,11 +13,11 @@ module ActionCable class EventedRedis < Base # :nodoc: @@mutex = Mutex.new - # Overwrite this factory method for EventMachine redis connections if you want to use a different Redis library than EM::Hiredis. + # Overwrite this factory method for EventMachine Redis connections if you want to use a different Redis connection library than EM::Hiredis. # This is needed, for example, when using Makara proxies for distributed Redis. cattr_accessor(:em_redis_connector) { ->(config) { EM::Hiredis.connect(config[:url]) } } - # Overwrite this factory method for redis connections if you want to use a different Redis library than Redis. + # Overwrite this factory method for Redis connections if you want to use a different Redis connection library than Redis. # This is needed, for example, when using Makara proxies for distributed Redis. cattr_accessor(:redis_connector) { ->(config) { ::Redis.new(url: config[:url]) } } diff --git a/actioncable/lib/rails/generators/channel/channel_generator.rb b/actioncable/lib/rails/generators/channel/channel_generator.rb index c5d398810a..6debe40c91 100644 --- a/actioncable/lib/rails/generators/channel/channel_generator.rb +++ b/actioncable/lib/rails/generators/channel/channel_generator.rb @@ -15,12 +15,29 @@ module Rails if options[:assets] template "assets/channel.coffee", File.join('app/assets/javascripts/channels', class_path, "#{file_name}.coffee") end + + generate_application_cable_files end protected def file_name @_file_name ||= super.gsub(/\_channel/i, '') end + + # FIXME: Change these files to symlinks once RubyGems 2.5.0 is required. + def generate_application_cable_files + return if self.behavior != :invoke + + files = [ + 'application_cable/channel.rb', + 'application_cable/connection.rb' + ] + + files.each do |name| + path = File.join('app/channels/', name) + template(name, path) if !File.exist?(path) + end + end end end end diff --git a/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb b/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb new file mode 100644 index 0000000000..d56fa30f4d --- /dev/null +++ b/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb @@ -0,0 +1,5 @@ +# Be sure to restart your server when you modify this file. Action Cable runs in a loop that does not support auto reloading. +module ApplicationCable + class Channel < ActionCable::Channel::Base + end +end diff --git a/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb b/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb new file mode 100644 index 0000000000..b4f41389ad --- /dev/null +++ b/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb @@ -0,0 +1,5 @@ +# Be sure to restart your server when you modify this file. Action Cable runs in a loop that does not support auto reloading. +module ApplicationCable + class Connection < ActionCable::Connection::Base + end +end diff --git a/actioncable/test/connection/base_test.rb b/actioncable/test/connection/base_test.rb index e2b017a9a1..3bef9e95a1 100644 --- a/actioncable/test/connection/base_test.rb +++ b/actioncable/test/connection/base_test.rb @@ -56,7 +56,7 @@ class ActionCable::Connection::BaseTest < ActionCable::TestCase run_in_eventmachine do connection = open_connection - connection.websocket.expects(:transmit).with(regexp_matches(/\_ping/)) + connection.websocket.expects(:transmit).with({ identifier: "_ping", type: "confirm_subscription" }.to_json) connection.message_buffer.expects(:process!) connection.process diff --git a/actioncable/test/subscription_adapter/base_test.rb b/actioncable/test/subscription_adapter/base_test.rb index 7a7ae131e6..256dce673f 100644 --- a/actioncable/test/subscription_adapter/base_test.rb +++ b/actioncable/test/subscription_adapter/base_test.rb @@ -43,7 +43,7 @@ class ActionCable::SubscriptionAdapter::BaseTest < ActionCable::TestCase assert_respond_to(SuccessAdapter.new(@server), :broadcast) - assert_nothing_raised NotImplementedError do + assert_nothing_raised do broadcast end end @@ -55,7 +55,7 @@ class ActionCable::SubscriptionAdapter::BaseTest < ActionCable::TestCase assert_respond_to(SuccessAdapter.new(@server), :subscribe) - assert_nothing_raised NotImplementedError do + assert_nothing_raised do subscribe end end @@ -66,7 +66,7 @@ class ActionCable::SubscriptionAdapter::BaseTest < ActionCable::TestCase assert_respond_to(SuccessAdapter.new(@server), :unsubscribe) - assert_nothing_raised NotImplementedError do + assert_nothing_raised do unsubscribe end end diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 1531f2b471..c281588864 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,9 @@ +* Add support to fragment cache in Action Mailer. + + Now you can use fragment caching in your mailers views. + + *Stan Lo* + * Reset `ActionMailer::Base.deliveries` after every test in `ActionDispatch::IntegrationTest`. diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 4259eb0bee..559cd06d91 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -430,6 +430,7 @@ module ActionMailer include AbstractController::Translation include AbstractController::AssetPaths include AbstractController::Callbacks + include AbstractController::Caching include ActionView::Layouts @@ -947,6 +948,18 @@ module ActionMailer container.add_part(part) end + # This and #instrument_name is for caching instrument + def instrument_payload(key) + { + mailer: mailer_name, + key: key + } + end + + def instrument_name + "action_mailer" + end + ActiveSupport.run_load_hooks(:action_mailer, self) end end diff --git a/actionmailer/lib/action_mailer/railtie.rb b/actionmailer/lib/action_mailer/railtie.rb index 22390c4953..215d0199af 100644 --- a/actionmailer/lib/action_mailer/railtie.rb +++ b/actionmailer/lib/action_mailer/railtie.rb @@ -25,6 +25,7 @@ module ActionMailer options.javascripts_dir ||= paths["public/javascripts"].first options.stylesheets_dir ||= paths["public/stylesheets"].first options.show_previews = Rails.env.development? if options.show_previews.nil? + options.cache_store ||= Rails.cache if options.show_previews options.preview_path ||= defined?(Rails.root) ? "#{Rails.root}/test/mailers/previews" : nil @@ -45,13 +46,6 @@ module ActionMailer options.each { |k,v| send("#{k}=", v) } - if options.show_previews - app.routes.prepend do - get '/rails/mailers' => "rails/mailers#index" - get '/rails/mailers/*path' => "rails/mailers#preview" - end - end - ActionDispatch::IntegrationTest.send :include, ActionMailer::TestCase::ClearTestDeliveries end end @@ -62,9 +56,18 @@ module ActionMailer end end - config.after_initialize do - if ActionMailer::Base.preview_path - ActiveSupport::Dependencies.autoload_paths << ActionMailer::Base.preview_path + config.after_initialize do |app| + options = app.config.action_mailer + + if options.show_previews + app.routes.prepend do + get '/rails/mailers' => "rails/mailers#index", internal: true + get '/rails/mailers/*path' => "rails/mailers#preview", internal: true + end + + if options.preview_path + ActiveSupport::Dependencies.autoload_paths << options.preview_path + end end end end diff --git a/actionmailer/test/caching_test.rb b/actionmailer/test/caching_test.rb new file mode 100644 index 0000000000..b4344eb167 --- /dev/null +++ b/actionmailer/test/caching_test.rb @@ -0,0 +1,229 @@ +require 'fileutils' +require 'abstract_unit' +require 'mailers/base_mailer' +require 'mailers/caching_mailer' + +CACHE_DIR = 'test_cache' +# Don't change '/../temp/' cavalierly or you might hose something you don't want hosed +FILE_STORE_PATH = File.join(File.dirname(__FILE__), '/../temp/', CACHE_DIR) + +class FragmentCachingMailer < ActionMailer::Base + abstract! + + def some_action; end +end + +class BaseCachingTest < ActiveSupport::TestCase + def setup + super + @store = ActiveSupport::Cache::MemoryStore.new + @mailer = FragmentCachingMailer.new + @mailer.perform_caching = true + @mailer.cache_store = @store + end + + def test_fragment_cache_key + assert_equal 'views/what a key', @mailer.fragment_cache_key('what a key') + end +end + +class FragmentCachingTest < BaseCachingTest + def test_read_fragment_with_caching_enabled + @store.write('views/name', 'value') + assert_equal 'value', @mailer.read_fragment('name') + end + + def test_read_fragment_with_caching_disabled + @mailer.perform_caching = false + @store.write('views/name', 'value') + assert_nil @mailer.read_fragment('name') + end + + def test_fragment_exist_with_caching_enabled + @store.write('views/name', 'value') + assert @mailer.fragment_exist?('name') + assert !@mailer.fragment_exist?('other_name') + end + + def test_fragment_exist_with_caching_disabled + @mailer.perform_caching = false + @store.write('views/name', 'value') + assert !@mailer.fragment_exist?('name') + assert !@mailer.fragment_exist?('other_name') + end + + def test_write_fragment_with_caching_enabled + assert_nil @store.read('views/name') + assert_equal 'value', @mailer.write_fragment('name', 'value') + assert_equal 'value', @store.read('views/name') + end + + def test_write_fragment_with_caching_disabled + assert_nil @store.read('views/name') + @mailer.perform_caching = false + assert_equal 'value', @mailer.write_fragment('name', 'value') + assert_nil @store.read('views/name') + end + + def test_expire_fragment_with_simple_key + @store.write('views/name', 'value') + @mailer.expire_fragment 'name' + assert_nil @store.read('views/name') + end + + def test_expire_fragment_with_regexp + @store.write('views/name', 'value') + @store.write('views/another_name', 'another_value') + @store.write('views/primalgrasp', 'will not expire ;-)') + + @mailer.expire_fragment(/name/) + + assert_nil @store.read('views/name') + assert_nil @store.read('views/another_name') + assert_equal 'will not expire ;-)', @store.read('views/primalgrasp') + end + + def test_fragment_for + @store.write('views/expensive', 'fragment content') + fragment_computed = false + + view_context = @mailer.view_context + + buffer = 'generated till now -> '.html_safe + buffer << view_context.send(:fragment_for, 'expensive') { fragment_computed = true } + + assert !fragment_computed + assert_equal 'generated till now -> fragment content', buffer + end + + def test_html_safety + assert_nil @store.read('views/name') + content = 'value'.html_safe + assert_equal content, @mailer.write_fragment('name', content) + + cached = @store.read('views/name') + assert_equal content, cached + assert_equal String, cached.class + + html_safe = @mailer.read_fragment('name') + assert_equal content, html_safe + assert html_safe.html_safe? + end +end + +class FunctionalFragmentCachingTest < BaseCachingTest + def setup + super + @store = ActiveSupport::Cache::MemoryStore.new + @mailer = CachingMailer.new + @mailer.perform_caching = true + @mailer.cache_store = @store + end + + def test_fragment_caching + email = @mailer.fragment_cache + expected_body = "\"Welcome\"" + + assert_match expected_body, email.body.encoded + assert_match "\"Welcome\"", + @store.read("views/caching/#{template_digest("caching_mailer/fragment_cache")}") + end + + def test_fragment_caching_in_partials + email = @mailer.fragment_cache_in_partials + assert_match(/Old fragment caching in a partial/, email.body.encoded) + + assert_match("Old fragment caching in a partial", + @store.read("views/caching/#{template_digest("caching_mailer/_partial")}")) + end + + def test_skip_fragment_cache_digesting + email = @mailer.skip_fragment_cache_digesting + expected_body = "No Digest" + + assert_match expected_body, email.body.encoded + assert_match expected_body, @store.read("views/no_digest") + end + + private + + def template_digest(name) + ActionView::Digestor.digest(name: name, finder: @mailer.lookup_context) + end +end + +class CacheHelperOutputBufferTest < BaseCachingTest + + class MockController + def read_fragment(name, options) + return false + end + + def write_fragment(name, fragment, options) + fragment + end + end + + def setup + super + end + + def test_output_buffer + output_buffer = ActionView::OutputBuffer.new + controller = MockController.new + cache_helper = Class.new do + def self.controller; end; + def self.output_buffer; end; + def self.output_buffer=; end; + end + cache_helper.extend(ActionView::Helpers::CacheHelper) + + cache_helper.stub :controller, controller do + cache_helper.stub :output_buffer, output_buffer do + assert_called_with cache_helper, :output_buffer=, [output_buffer.class.new(output_buffer)] do + assert_nothing_raised do + cache_helper.send :fragment_for, 'Test fragment name', 'Test fragment', &Proc.new{ nil } + end + end + end + end + end + + def test_safe_buffer + output_buffer = ActiveSupport::SafeBuffer.new + controller = MockController.new + cache_helper = Class.new do + def self.controller; end; + def self.output_buffer; end; + def self.output_buffer=; end; + end + cache_helper.extend(ActionView::Helpers::CacheHelper) + + cache_helper.stub :controller, controller do + cache_helper.stub :output_buffer, output_buffer do + assert_called_with cache_helper, :output_buffer=, [output_buffer.class.new(output_buffer)] do + assert_nothing_raised do + cache_helper.send :fragment_for, 'Test fragment name', 'Test fragment', &Proc.new{ nil } + end + end + end + end + end +end + +class ViewCacheDependencyTest < BaseCachingTest + class NoDependenciesMailer < ActionMailer::Base + end + class HasDependenciesMailer < ActionMailer::Base + view_cache_dependency { "trombone" } + view_cache_dependency { "flute" } + end + + def test_view_cache_dependencies_are_empty_by_default + assert NoDependenciesMailer.new.view_cache_dependencies.empty? + end + + def test_view_cache_dependencies_are_listed_in_declaration_order + assert_equal %w(trombone flute), HasDependenciesMailer.new.view_cache_dependencies + end +end diff --git a/actionmailer/test/fixtures/caching_mailer/_partial.html.erb b/actionmailer/test/fixtures/caching_mailer/_partial.html.erb new file mode 100644 index 0000000000..8e965f52b4 --- /dev/null +++ b/actionmailer/test/fixtures/caching_mailer/_partial.html.erb @@ -0,0 +1,3 @@ +<% cache :caching do %> + Old fragment caching in a partial +<% end %> diff --git a/actionmailer/test/fixtures/caching_mailer/fragment_cache.html.erb b/actionmailer/test/fixtures/caching_mailer/fragment_cache.html.erb new file mode 100644 index 0000000000..90189627da --- /dev/null +++ b/actionmailer/test/fixtures/caching_mailer/fragment_cache.html.erb @@ -0,0 +1,3 @@ +<% cache :caching do %> +"Welcome" +<% end %> diff --git a/actionmailer/test/fixtures/caching_mailer/fragment_cache_in_partials.html.erb b/actionmailer/test/fixtures/caching_mailer/fragment_cache_in_partials.html.erb new file mode 100644 index 0000000000..2957d083e8 --- /dev/null +++ b/actionmailer/test/fixtures/caching_mailer/fragment_cache_in_partials.html.erb @@ -0,0 +1 @@ +<%= render "partial" %> diff --git a/actionmailer/test/fixtures/caching_mailer/skip_fragment_cache_digesting.html.erb b/actionmailer/test/fixtures/caching_mailer/skip_fragment_cache_digesting.html.erb new file mode 100644 index 0000000000..0d52429a81 --- /dev/null +++ b/actionmailer/test/fixtures/caching_mailer/skip_fragment_cache_digesting.html.erb @@ -0,0 +1,3 @@ +<%= cache :no_digest, skip_digest: true do %> + No Digest +<% end %> diff --git a/actionmailer/test/mailers/caching_mailer.rb b/actionmailer/test/mailers/caching_mailer.rb new file mode 100644 index 0000000000..345d267a36 --- /dev/null +++ b/actionmailer/test/mailers/caching_mailer.rb @@ -0,0 +1,15 @@ +class CachingMailer < ActionMailer::Base + self.mailer_name = "caching_mailer" + + def fragment_cache + mail(subject: "welcome", template_name: "fragment_cache") + end + + def fragment_cache_in_partials + mail(subject: "welcome", template_name: "fragment_cache_in_partials") + end + + def skip_fragment_cache_digesting + mail(subject: "welcome", template_name: "skip_fragment_cache_digesting") + end +end diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index d473ab427d..4425a90798 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,12 @@ +* Update session to have indifferent access across multiple requests + + session[:deep][:hash] = "Magic" + + session[:deep][:hash] == "Magic" + session[:deep]["hash"] == "Magic" + + *Tom Prats* + * Add application/gzip as a default mime type. *Mehmet Emin İNAÇ* diff --git a/actionpack/lib/abstract_controller.rb b/actionpack/lib/abstract_controller.rb index 56c4033387..1e57cbaac4 100644 --- a/actionpack/lib/abstract_controller.rb +++ b/actionpack/lib/abstract_controller.rb @@ -6,6 +6,7 @@ module AbstractController extend ActiveSupport::Autoload autoload :Base + autoload :Caching autoload :Callbacks autoload :Collector autoload :DoubleRenderError, "abstract_controller/rendering" @@ -15,4 +16,9 @@ module AbstractController autoload :Translation autoload :AssetPaths autoload :UrlFor + + def self.eager_load! + super + AbstractController::Caching.eager_load! + end end diff --git a/actionpack/lib/abstract_controller/caching.rb b/actionpack/lib/abstract_controller/caching.rb new file mode 100644 index 0000000000..0dea50889a --- /dev/null +++ b/actionpack/lib/abstract_controller/caching.rb @@ -0,0 +1,62 @@ +module AbstractController + module Caching + extend ActiveSupport::Concern + extend ActiveSupport::Autoload + + eager_autoload do + autoload :Fragments + end + + module ConfigMethods + def cache_store + config.cache_store + end + + def cache_store=(store) + config.cache_store = ActiveSupport::Cache.lookup_store(store) + end + + private + def cache_configured? + perform_caching && cache_store + end + end + + include ConfigMethods + include AbstractController::Caching::Fragments + + included do + extend ConfigMethods + + config_accessor :default_static_extension + self.default_static_extension ||= '.html' + + config_accessor :perform_caching + self.perform_caching = true if perform_caching.nil? + + class_attribute :_view_cache_dependencies + self._view_cache_dependencies = [] + helper_method :view_cache_dependencies if respond_to?(:helper_method) + end + + module ClassMethods + def view_cache_dependency(&dependency) + self._view_cache_dependencies += [dependency] + end + end + + def view_cache_dependencies + self.class._view_cache_dependencies.map { |dep| instance_exec(&dep) }.compact + end + + protected + # Convenience accessor. + def cache(key, options = {}, &block) + if cache_configured? + cache_store.fetch(ActiveSupport::Cache.expand_cache_key(key, :controller), options, &block) + else + yield + end + end + end +end diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/abstract_controller/caching/fragments.rb index b9ad51a9cf..3257a731ed 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/abstract_controller/caching/fragments.rb @@ -1,4 +1,4 @@ -module ActionController +module AbstractController module Caching # Fragment caching is used for caching various blocks within # views without caching the entire action as a whole. This is @@ -135,13 +135,8 @@ module ActionController end def instrument_fragment_cache(name, key) # :nodoc: - payload = { - controller: controller_name, - action: action_name, - key: key - } - - ActiveSupport::Notifications.instrument("#{name}.action_controller", payload) { yield } + payload = instrument_payload(key) + ActiveSupport::Notifications.instrument("#{name}.#{instrument_name}", payload) { yield } end end end diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 40f33a9de0..62f5905205 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -9,12 +9,15 @@ module ActionController autoload :API autoload :Base - autoload :Caching autoload :Metal autoload :Middleware autoload :Renderer autoload :FormBuilder + eager_autoload do + autoload :Caching + end + autoload_under "metal" do autoload :ConditionalGet autoload :Cookies @@ -47,11 +50,6 @@ module ActionController autoload :TestCase, 'action_controller/test_case' autoload :TemplateAssertions, 'action_controller/test_case' - - def self.eager_load! - super - ActionController::Caching.eager_load! - end end # Common Active Support usage in Action Controller diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index 0b8fa2ea09..a9a8508abc 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -1,6 +1,3 @@ -require 'fileutils' -require 'uri' - module ActionController # \Caching is a cheap way of speeding up slow applications by keeping the result of # calculations, renderings, and database calls around for subsequent requests. @@ -23,65 +20,25 @@ module ActionController # config.action_controller.cache_store = :mem_cache_store, Memcached::Rails.new('localhost:11211') # config.action_controller.cache_store = MyOwnStore.new('parameter') module Caching - extend ActiveSupport::Concern extend ActiveSupport::Autoload - - eager_autoload do - autoload :Fragments - end - - module ConfigMethods - def cache_store - config.cache_store - end - - def cache_store=(store) - config.cache_store = ActiveSupport::Cache.lookup_store(store) - end - - private - def cache_configured? - perform_caching && cache_store - end - end - - include AbstractController::Callbacks - - include ConfigMethods - include Fragments + extend ActiveSupport::Concern included do - extend ConfigMethods - - config_accessor :default_static_extension - self.default_static_extension ||= '.html' - - config_accessor :perform_caching - self.perform_caching = true if perform_caching.nil? - - class_attribute :_view_cache_dependencies - self._view_cache_dependencies = [] - helper_method :view_cache_dependencies if respond_to?(:helper_method) + include AbstractController::Caching end - module ClassMethods - def view_cache_dependency(&dependency) - self._view_cache_dependencies += [dependency] - end - end + private - def view_cache_dependencies - self.class._view_cache_dependencies.map { |dep| instance_exec(&dep) }.compact - end + def instrument_payload(key) + { + controller: controller_name, + action: action_name, + key: key + } + end - protected - # Convenience accessor. - def cache(key, options = {}, &block) - if cache_configured? - cache_store.fetch(ActiveSupport::Cache.expand_cache_key(key, :controller), options, &block) - else - yield - end + def instrument_name + "action_controller" end end end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 6586985ff5..b2f0b382b9 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -379,7 +379,8 @@ module ActionController #:nodoc: def xor_byte_strings(s1, s2) s2_bytes = s2.bytes - s1.bytes.map.with_index { |c1, i| c1 ^ s2_bytes[i] }.pack('c*') + s1.each_byte.with_index { |c1, i| s2_bytes[i] ^= c1 } + s2_bytes.pack('C*') end # The form's authenticity parameter. Override to provide your own. diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 0c4b661214..6548ce326b 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -52,7 +52,7 @@ module ActionController self.session = session self.session_options = TestSession::DEFAULT_OPTIONS @custom_param_parsers = { - Mime[:xml] => lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } + xml: lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } } end @@ -105,7 +105,7 @@ module ActionController when :url_encoded_form data = non_path_parameters.to_query else - @custom_param_parsers[content_mime_type] = ->(_) { non_path_parameters } + @custom_param_parsers[content_mime_type.symbol] = ->(_) { non_path_parameters } data = non_path_parameters.to_query end end @@ -176,7 +176,7 @@ module ActionController def initialize(session = {}) super(nil, nil) @id = SecureRandom.hex(16) - @data = stringify_keys(session) + @data = session.with_indifferent_access @loaded = true end diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index cca7376ffa..ff5031d7d5 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -1,22 +1,31 @@ module ActionDispatch module Http module Parameters + extend ActiveSupport::Concern + PARAMETERS_KEY = 'action_dispatch.request.path_parameters' DEFAULT_PARSERS = { - Mime[:json] => lambda { |raw_post| + Mime[:json].symbol => -> (raw_post) { data = ActiveSupport::JSON.decode(raw_post) data.is_a?(Hash) ? data : {:_json => data} } } - def self.included(klass) - class << klass - attr_accessor :parameter_parsers + included do + class << self + attr_reader :parameter_parsers end - klass.parameter_parsers = DEFAULT_PARSERS + self.parameter_parsers = DEFAULT_PARSERS end + + module ClassMethods + def parameter_parsers=(parsers) # :nodoc: + @parameter_parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } + end + end + # Returns both GET and POST \parameters in a single hash. def parameters params = get_header("action_dispatch.request.parameters") @@ -51,7 +60,7 @@ module ActionDispatch def parse_formatted_parameters(parsers) return yield if content_length.zero? - strategy = parsers.fetch(content_mime_type) { return yield } + strategy = parsers.fetch(content_mime_type.symbol) { return yield } begin strategy.call(raw_post) diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 5427425ef7..316a9f08b7 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -403,6 +403,10 @@ module ActionDispatch def commit_flash end + def ssl? + super || scheme == 'wss'.freeze + end + private def check_method(name) HTTP_METHOD_LOOKUP[name] || raise(ActionController::UnknownHttpMethod, "#{name}, accepted HTTP methods are #{HTTP_METHODS[0...-1].join(', ')}, and #{HTTP_METHODS[-1]}") diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 35c2b1b86e..fee08fc3db 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -3,7 +3,7 @@ module ActionDispatch class Route # :nodoc: attr_reader :app, :path, :defaults, :name, :precedence - attr_reader :constraints + attr_reader :constraints, :internal alias :conditions :constraints module VerbMatchers @@ -55,7 +55,7 @@ module ActionDispatch ## # +path+ is a path constraint. # +constraints+ is a hash of constraints to be applied to this route. - def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match, precedence) + def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match, precedence, internal = false) @name = name @app = app @path = path @@ -70,6 +70,7 @@ module ActionDispatch @decorated_ast = nil @precedence = precedence @path_formatter = @path.build_formatter + @internal = internal end def ast diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index c2a4f46e67..5841c978af 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -37,6 +37,7 @@ module ActionDispatch # The +parsers+ argument can take Hash of parsers where key is identifying # content mime type, and value is a lambda that is going to process data. def self.new(app, parsers = {}) + parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } ActionDispatch::Request.parameter_parsers = ActionDispatch::Request::DEFAULT_PARSERS.merge(parsers) app end diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index 42890225fa..38d0da3e67 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -9,7 +9,7 @@ module ActionDispatch # Singleton object used to determine if an optional param wasn't specified Unspecified = Object.new - + # Creates a session hash, merging the properties of the previous session if any def self.create(store, req, default_options) session_was = find req @@ -61,7 +61,7 @@ module ActionDispatch def initialize(by, req) @by = by @req = req - @delegate = {} + @delegate = {}.with_indifferent_access @loaded = false @exists = nil # we haven't checked yet end @@ -88,13 +88,13 @@ module ActionDispatch # nil if the given key is not found in the session. def [](key) load_for_read! - @delegate[key.to_s] + @delegate[key] end # Returns true if the session has the given key or false. def has_key?(key) load_for_read! - @delegate.key?(key.to_s) + @delegate.key?(key) end alias :key? :has_key? alias :include? :has_key? @@ -112,7 +112,7 @@ module ActionDispatch # Writes given value to given key of the session. def []=(key, value) load_for_write! - @delegate[key.to_s] = value + @delegate[key] = value end # Clears the session. @@ -139,13 +139,13 @@ module ActionDispatch # # => {"session_id"=>"e29b9ea315edf98aad94cc78c34cc9b2", "foo" => "bar"} def update(hash) load_for_write! - @delegate.update stringify_keys(hash) + @delegate.update hash end # Deletes given key from the session. def delete(key) load_for_write! - @delegate.delete key.to_s + @delegate.delete key end # Returns value of the given key from the session, or raises +KeyError+ @@ -165,9 +165,9 @@ module ActionDispatch def fetch(key, default=Unspecified, &block) load_for_read! if default == Unspecified - @delegate.fetch(key.to_s, &block) + @delegate.fetch(key, &block) else - @delegate.fetch(key.to_s, default, &block) + @delegate.fetch(key, default, &block) end end @@ -211,15 +211,9 @@ module ActionDispatch def load! id, session = @by.load_session @req options[:id] = id - @delegate.replace(stringify_keys(session)) + @delegate.replace(session) @loaded = true end - - def stringify_keys(other) - other.each_with_object({}) { |(key, value), hash| - hash[key.to_s] = value - } - end end end end diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 79f9283f83..dcf800b215 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -159,7 +159,7 @@ module ActionDispatch # # controller 'geocode' do # get 'geocode/:postalcode' => :show, constraints: { - # postalcode: /# Postcode format + # postalcode: /# Postalcode format # \d{5} #Prefix # (-\d{4})? #Suffix # /x diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 983f1daeb3..6f651a5689 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -41,7 +41,7 @@ module ActionDispatch end def internal? - controller.to_s =~ %r{\Arails/(info|mailers|welcome)} + internal end def engine? diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index afbaa45d20..16b430c36e 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -107,6 +107,7 @@ module ActionDispatch @ast = ast @anchor = anchor @via = via + @internal = options[:internal] path_params = ast.find_all(&:symbol?).map(&:to_sym) @@ -148,7 +149,8 @@ module ActionDispatch required_defaults, defaults, request_method, - precedence) + precedence, + @internal) route end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 7556f984f2..754ac144cc 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -381,19 +381,14 @@ class CollectionCacheController < ActionController::Base render 'index' end - def index_explicit_render + def index_explicit_render_in_controller @customers = [Customer.new('david', 1)] - render partial: 'customers/customer', collection: @customers + render partial: 'customers/customer', collection: @customers, cached: true end def index_with_comment @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' } + render partial: 'customers/commented_customer', collection: @customers, as: :customer, cached: true end end @@ -404,7 +399,7 @@ class AutomaticCollectionCacheTest < ActionController::TestCase @controller.perform_caching = true @controller.partial_rendered_times = 0 @controller.cache_store = ActiveSupport::Cache::MemoryStore.new - ActionView::PartialRenderer.collection_cache = @controller.cache_store + ActionView::PartialRenderer.collection_cache = ActiveSupport::Cache::MemoryStore.new end def test_collection_fetches_cached_views @@ -427,7 +422,7 @@ class AutomaticCollectionCacheTest < ActionController::TestCase end def test_explicit_render_call_with_options - get :index_explicit_render + get :index_explicit_render_in_controller assert_select ':root', "david, 1" end @@ -440,12 +435,6 @@ class AutomaticCollectionCacheTest < ActionController::TestCase 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, diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb index 22f1cc7c22..03a9c9ae78 100644 --- a/actionpack/test/controller/force_ssl_test.rb +++ b/actionpack/test/controller/force_ssl_test.rb @@ -322,3 +322,12 @@ class RedirectToSSLTest < ActionController::TestCase assert_equal 'ihaz', response.body end end + +class ForceSSLControllerLevelTest < ActionController::TestCase + def test_no_redirect_websocket_ssl_request + request.env['rack.url_scheme'] = 'wss' + request.env['Upgrade'] = 'websocket' + get :cheeseburger + assert_response 200 + end +end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index ea50f05f4d..6277407ff7 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -390,7 +390,7 @@ class IntegrationTestUsesCorrectClass < ActionDispatch::IntegrationTest reset! %w( get post head patch put delete ).each do |verb| - assert_nothing_raised("'#{verb}' should use integration test methods") { __send__(verb, '/') } + assert_nothing_raised { __send__(verb, '/') } end end end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 1984ad8825..f7dcbc1984 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -133,7 +133,11 @@ class PerFormTokensController < ActionController::Base self.per_form_csrf_tokens = true def index - render inline: "<%= form_tag (params[:form_path] || '/per_form_tokens/post_one'), method: (params[:form_method] || :post) %>" + render inline: "<%= form_tag (params[:form_path] || '/per_form_tokens/post_one'), method: params[:form_method] %>" + end + + def button_to + render inline: "<%= button_to 'Button', (params[:form_path] || '/per_form_tokens/post_one'), method: params[:form_method] %>" end def post_one @@ -652,15 +656,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_accepts_token_for_correct_path_and_method get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one' @@ -673,15 +671,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_rejects_token_for_incorrect_path get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_two' @@ -693,15 +685,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_rejects_token_for_incorrect_method get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one' @@ -710,6 +696,50 @@ class PerFormTokensControllerTest < ActionController::TestCase end end + def test_rejects_token_for_incorrect_method_button_to + get :button_to, params: { form_method: 'delete' } + + form_token = assert_presence_and_fetch_form_csrf_token + + assert_matches_session_token_on_server form_token, 'delete' + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_raises(ActionController::InvalidAuthenticityToken) do + patch :post_one, params: { custom_authenticity_token: form_token } + end + end + + test "Accepts proper token for implicit post method on button_to tag" do + get :button_to + + form_token = assert_presence_and_fetch_form_csrf_token + + assert_matches_session_token_on_server form_token, 'post' + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_nothing_raised do + post :post_one, params: { custom_authenticity_token: form_token } + end + end + + %w{delete post patch}.each do |verb| + test "Accepts proper token for #{verb} method on button_to tag" do + get :button_to, params: { form_method: verb } + + form_token = assert_presence_and_fetch_form_csrf_token + + assert_matches_session_token_on_server form_token, verb + + # This is required because PATH_INFO isn't reset between requests. + @request.env['PATH_INFO'] = '/per_form_tokens/post_one' + assert_nothing_raised do + send verb, :post_one, params: { custom_authenticity_token: form_token } + end + end + end + def test_accepts_global_csrf_token get :index @@ -726,15 +756,9 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_ignores_params get :index, params: {form_path: '/per_form_tokens/post_one?foo=bar'} - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) - expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', 'post') - assert_equal expected, actual + assert_matches_session_token_on_server form_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one?foo=baz' @@ -747,11 +771,7 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_ignores_trailing_slash_during_generation get :index, params: {form_path: '/per_form_tokens/post_one/'} - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one' @@ -764,11 +784,7 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_ignores_trailing_slash_during_validation get :index - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end + form_token = assert_presence_and_fetch_form_csrf_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one/' @@ -781,12 +797,7 @@ class PerFormTokensControllerTest < ActionController::TestCase def test_method_is_case_insensitive get :index, params: {form_method: "POST"} - form_token = nil - assert_select 'input[name=custom_authenticity_token]' do |elts| - form_token = elts.first['value'] - assert_not_nil form_token - end - + form_token = assert_presence_and_fetch_form_csrf_token # This is required because PATH_INFO isn't reset between requests. @request.env['PATH_INFO'] = '/per_form_tokens/post_one/' assert_nothing_raised do @@ -794,4 +805,19 @@ class PerFormTokensControllerTest < ActionController::TestCase end assert_response :success end + + private + def assert_presence_and_fetch_form_csrf_token + assert_select 'input[name="custom_authenticity_token"]' do |input| + form_csrf_token = input.first['value'] + assert_not_nil form_csrf_token + return form_csrf_token + end + end + + def assert_matches_session_token_on_server(form_token, method = 'post') + actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) + expected = @controller.send(:per_form_csrf_token, session, '/per_form_tokens/post_one', method) + assert_equal expected, actual + end end diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index 6d377c4691..daf17558aa 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -99,7 +99,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_parsing_json_doesnot_rescue_exception req = Class.new(ActionDispatch::Request) do def params_parsers - { Mime[:json] => Proc.new { |data| raise Interrupt } } + { json: Proc.new { |data| raise Interrupt } } end def content_length; get_header('rack.input').length; end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index 64801bff39..3655c7f570 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -150,6 +150,34 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest ) end + test "parses json params after custom json mime type registered" do + begin + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w(application/vnd.api+json) + assert_parses( + {"user" => {"username" => "meinac"}, "username" => "meinac"}, + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/json' } + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) + end + end + + test "parses json params after custom json mime type registered with synonym" do + begin + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w(application/vnd.api+json) + assert_parses( + {"user" => {"username" => "meinac"}, "username" => "meinac"}, + "{\"username\": \"meinac\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) + end + end + private def assert_parses(expected, actual, headers = {}) with_test_routing(UsersController) do diff --git a/actionpack/test/dispatch/request/session_test.rb b/actionpack/test/dispatch/request/session_test.rb index 7dcbcc5c21..3433d82791 100644 --- a/actionpack/test/dispatch/request/session_test.rb +++ b/actionpack/test/dispatch/request/session_test.rb @@ -105,6 +105,16 @@ module ActionDispatch end end + def test_indifferent_access + s = Session.create(store, req, {}) + + s[:one] = { test: "deep" } + s[:two] = { "test" => "deep" } + + assert_equal 'deep', s[:one]["test"] + assert_equal 'deep', s[:two][:test] + end + private def store Class.new { diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb index f72a87b994..fd85cc6e9f 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -389,6 +389,29 @@ module ActionDispatch ], output end + def test_displaying_routes_for_internal_engines + engine = Class.new(Rails::Engine) do + def self.inspect + "Blog::Engine" + end + end + engine.routes.draw do + get '/cart', to: 'cart#show' + post '/cart', to: 'cart#create' + patch '/cart', to: 'cart#update' + end + + output = draw do + get '/custom/assets', to: 'custom_assets#show' + mount engine => "/blog", as: "blog", internal: true + end + + assert_equal [ + " Prefix Verb URI Pattern Controller#Action", + "custom_assets GET /custom/assets(.:format) custom_assets#show", + ], output + end + end end end diff --git a/actionpack/test/dispatch/session/abstract_store_test.rb b/actionpack/test/dispatch/session/abstract_store_test.rb index d38d1bbce6..c9ce5cad42 100644 --- a/actionpack/test/dispatch/session/abstract_store_test.rb +++ b/actionpack/test/dispatch/session/abstract_store_test.rb @@ -46,6 +46,22 @@ module ActionDispatch assert_equal session.to_hash, session1.to_hash end + def test_previous_session_has_indifferent_access + env = {} + as = MemoryStore.new app + as.call(env) + + assert @env + session = Request::Session.find ActionDispatch::Request.new @env + session[:foo] = { bar: "baz" } + + as.call(@env) + session = Request::Session.find ActionDispatch::Request.new @env + + assert_equal session[:foo][:bar], "baz" + assert_equal session[:foo]["bar"], "baz" + end + private def app(&block) @env = nil diff --git a/actionpack/test/dispatch/session/cache_store_test.rb b/actionpack/test/dispatch/session/cache_store_test.rb index dbb996973d..b911392cf1 100644 --- a/actionpack/test/dispatch/session/cache_store_test.rb +++ b/actionpack/test/dispatch/session/cache_store_test.rb @@ -12,6 +12,11 @@ class CacheStoreTest < ActionDispatch::IntegrationTest head :ok end + def set_deep_session_value + session[:foo] = { bar: "baz" } + head :ok + end + def set_serialized_session_value session[:foo] = SessionAutoloadTest::Foo.new head :ok @@ -21,6 +26,14 @@ class CacheStoreTest < ActionDispatch::IntegrationTest render plain: "foo: #{session[:foo].inspect}" end + def get_deep_session_value_with_symbol + render plain: "foo: { bar: #{session[:foo][:bar].inspect} }" + end + + def get_deep_session_value_with_string + render plain: "foo: { \"bar\" => #{session[:foo]["bar"].inspect} }" + end + def get_session_id render plain: "#{request.session.id}" end @@ -160,6 +173,22 @@ class CacheStoreTest < ActionDispatch::IntegrationTest end end + def test_previous_session_has_indifferent_access + with_test_route_set do + get '/set_deep_session_value' + assert_response :success + assert cookies['_session_id'] + + get '/get_deep_session_value_with_symbol' + assert_response :success + assert_equal 'foo: { bar: "baz" }', response.body + + get '/get_deep_session_value_with_string' + assert_response :success + assert_equal 'foo: { "bar" => "baz" }', response.body + end + end + private def with_test_route_set with_routing do |set| diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index f07e215e3a..71402b021a 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -24,10 +24,23 @@ class CookieStoreTest < ActionDispatch::IntegrationTest render plain: Rack::Utils.escape(Verifier.generate(session.to_hash)) end + def set_deep_session_value + session[:foo] = { bar: "baz" } + render plain: Rack::Utils.escape(Verifier.generate(session.to_hash)) + end + def get_session_value render plain: "foo: #{session[:foo].inspect}" end + def get_deep_session_value_with_symbol + render plain: "foo: { bar: #{session[:foo][:bar].inspect} }" + end + + def get_deep_session_value_with_string + render plain: "foo: { \"bar\" => #{session[:foo]["bar"].inspect} }" + end + def get_session_id render plain: "id: #{request.session.id}" end @@ -81,6 +94,15 @@ class CookieStoreTest < ActionDispatch::IntegrationTest end end + def test_session_indifferent_access + with_test_route_set do + cookies[SessionKey] = SignedBar + get '/get_session_value' + assert_response :success + assert_equal 'foo: "bar"', response.body + end + end + def test_getting_session_id with_test_route_set do cookies[SessionKey] = SignedBar @@ -332,6 +354,18 @@ class CookieStoreTest < ActionDispatch::IntegrationTest end end + def test_previous_session_has_indifferent_access + with_test_route_set do + get '/set_deep_session_value' + + get '/get_deep_session_value_with_symbol' + assert_equal 'foo: { bar: "baz" }', response.body + + get '/get_deep_session_value_with_string' + assert_equal 'foo: { "bar" => "baz" }', response.body + end + end + private # Overwrite get to send SessionSecret in env hash diff --git a/actionpack/test/dispatch/session/mem_cache_store_test.rb b/actionpack/test/dispatch/session/mem_cache_store_test.rb index 3fed9bad4f..2e6b42856f 100644 --- a/actionpack/test/dispatch/session/mem_cache_store_test.rb +++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb @@ -13,6 +13,11 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest head :ok end + def set_deep_session_value + session[:foo] = { bar: "baz" } + head :ok + end + def set_serialized_session_value session[:foo] = SessionAutoloadTest::Foo.new head :ok @@ -22,6 +27,14 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest render plain: "foo: #{session[:foo].inspect}" end + def get_deep_session_value_with_symbol + render plain: "foo: { bar: #{session[:foo][:bar].inspect} }" + end + + def get_deep_session_value_with_string + render plain: "foo: { \"bar\" => #{session[:foo]["bar"].inspect} }" + end + def get_session_id render plain: "#{request.session.id}" end @@ -179,6 +192,24 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest rescue Dalli::RingError => ex skip ex.message, ex.backtrace end + + def test_previous_session_has_indifferent_access + with_test_route_set do + get '/set_deep_session_value' + assert_response :success + assert cookies['_session_id'] + + get '/get_deep_session_value_with_symbol' + assert_response :success + assert_equal 'foo: { bar: "baz" }', response.body + + get '/get_deep_session_value_with_string' + assert_response :success + assert_equal 'foo: { "bar" => "baz" }', response.body + end + rescue Dalli::RingError => ex + skip ex.message, ex.backtrace + end rescue LoadError, RuntimeError, Dalli::DalliError $stderr.puts "Skipping MemCacheStoreTest tests. Start memcached and try again." end diff --git a/actionpack/test/dispatch/session/test_session_test.rb b/actionpack/test/dispatch/session/test_session_test.rb index 3e61d123e3..332c2ae3c8 100644 --- a/actionpack/test/dispatch/session/test_session_test.rb +++ b/actionpack/test/dispatch/session/test_session_test.rb @@ -60,4 +60,11 @@ class ActionController::TestSessionTest < ActiveSupport::TestCase session = ActionController::TestSession.new(one: '1') assert_equal(2, session.fetch('2') { |key| key.to_i }) end + + def test_fetch_returns_indifferent_access + session = ActionController::TestSession.new(three: { two: '1' }) + three = session.fetch(:three) + assert_equal('1', three[:two]) + assert_equal('1', three["two"]) + end end diff --git a/actionpack/test/fixtures/collection_cache/index.html.erb b/actionpack/test/fixtures/collection_cache/index.html.erb index 521b1450df..853e501ab4 100644 --- a/actionpack/test/fixtures/collection_cache/index.html.erb +++ b/actionpack/test/fixtures/collection_cache/index.html.erb @@ -1 +1 @@ -<%= render @customers %>
\ No newline at end of file +<%= render partial: 'customers/customer', collection: @customers, cached: true %> diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 461a26b10b..bd7ce14e04 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,4 +1,4 @@ -* Fix `button_to`'s `:params` option support to correctly generated input names for nested hashes/arrays. +* Add support for nested hashes/arrays to `:params` option of `button_to` helper. *James Coleman* @@ -197,16 +197,18 @@ *Ulisses Almeida* -* Collection rendering automatically caches and fetches multiple partials. +* Collection rendering can cache and fetch multiple partials at once. Collections rendered as: ```ruby - <%= render @notifications %> - <%= render partial: 'notifications/notification', collection: @notifications, as: :notification %> + <%= render partial: 'notifications/notification', collection: @notifications, as: :notification, cached: true %> ``` - will now read several partials from cache at once, if the template starts with a cache call: + will read several partials from cache at once. The templates in the collection + that haven't been cached already will automatically be written to cache. Works + great alongside individual template fragment caching. For instance if the + template the collection renders is cached like: ```ruby # notifications/_notification.html.erb @@ -215,6 +217,9 @@ <% end %> ``` + Then any collection renders shares that cache when attempting to read multiple + ones at once. + *Kasper Timm Hansen* * Fixed a dependency tracker bug that caused template dependencies not diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb index 401f398721..4c7c4b91c6 100644 --- a/actionview/lib/action_view/helpers/cache_helper.rb +++ b/actionview/lib/action_view/helpers/cache_helper.rb @@ -126,44 +126,29 @@ module ActionView # # Now all you have to do is change that timestamp when the helper method changes. # - # === Automatic Collection Caching + # === Collection Caching # - # When rendering collections such as: + # When rendering a collection of objects that each use the same partial, a `cached` + # option can be passed. + # For collections rendered such: # - # <%= render @notifications %> - # <%= render partial: 'notifications/notification', collection: @notifications %> + # <%= render partial: 'notifications/notification', collection: @notifications, cached: true %> # - # If the notifications/_notification partial starts with a cache call as: + # The `cached: true` will make Action View's rendering read several templates + # from cache at once instead of one call per template. # - # <% cache notification do %> - # <%= notification.name %> - # <% end %> - # - # The collection can then automatically use any cached renders for that - # template by reading them at once instead of one by one. - # - # See ActionView::Template::Handlers::ERB.resource_cache_call_pattern for - # more information on what cache calls make a template eligible for this - # collection caching. - # - # The automatic cache multi read can be turned off like so: + # Templates in the collection not already cached are written to cache. # - # <%= render @notifications, cache: false %> + # Works great alongside individual template fragment caching. + # For instance if the template the collection renders is cached like: # - # === Explicit Collection Caching - # - # If the partial template doesn't start with a clean cache call as - # mentioned above, you can still benefit from collection caching by - # adding a special comment format anywhere in the template, like: - # - # <%# Template Collection: notification %> - # <% my_helper_that_calls_cache(some_arg, notification) do %> - # <%= notification.name %> + # # notifications/_notification.html.erb + # <% cache notification do %> + # <%# ... %> # <% end %> # - # The pattern used to match these is <tt>/# Template Collection: (\S+)/</tt>, - # so it's important that you type it out just so. - # You can only declare one collection in a partial template file. + # Any collection renders will find those cached templates when attempting + # to read multiple templates at once. def cache(name = {}, options = {}, &block) if controller.respond_to?(:perform_caching) && controller.perform_caching name_options = options.slice(:skip_digest, :virtual_path) diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index 55dac74d00..82e9ace428 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -862,13 +862,13 @@ module ActionView def extra_tags_for_form(html_options) authenticity_token = html_options.delete("authenticity_token") - method = html_options.delete("method").to_s + method = html_options.delete("method").to_s.downcase method_tag = case method - when /^get$/i # must be case-insensitive, but can't use downcase as might be nil + when 'get' html_options["method"] = "get" '' - when /^post$/i, "", nil + when 'post', '' html_options["method"] = "post" token_tag(authenticity_token, form_options: { action: html_options["action"], diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index 87218821ed..ab67923376 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -312,7 +312,8 @@ module ActionView form_options[:'data-remote'] = true if remote request_token_tag = if form_method == 'post' - token_tag(nil, form_options: form_options) + request_method = method.empty? ? 'post' : method + token_tag(nil, form_options: { action: url, method: request_method }) else '' end diff --git a/actionview/lib/action_view/log_subscriber.rb b/actionview/lib/action_view/log_subscriber.rb index 9047dbdd85..aa38db2a3a 100644 --- a/actionview/lib/action_view/log_subscriber.rb +++ b/actionview/lib/action_view/log_subscriber.rb @@ -20,7 +20,15 @@ module ActionView end end alias :render_partial :render_template - alias :render_collection :render_template + + def render_collection(event) + identifier = event.payload[:identifier] || 'templates' + + info do + " Rendered collection of #{from_rails_root(identifier)}" \ + " #{render_count(event.payload)} (#{event.duration.round(1)}ms)" + end + end def logger ActionView::Base.logger @@ -38,6 +46,14 @@ module ActionView def rails_root @root ||= "#{Rails.root}/" end + + def render_count(payload) + if payload[:cache_hits] + "[#{payload[:cache_hits]} / #{payload[:count]} cache hits]" + else + "[#{payload[:count]} times]" + end + end end end diff --git a/actionview/lib/action_view/renderer/abstract_renderer.rb b/actionview/lib/action_view/renderer/abstract_renderer.rb index aa77a77acf..23e672a95f 100644 --- a/actionview/lib/action_view/renderer/abstract_renderer.rb +++ b/actionview/lib/action_view/renderer/abstract_renderer.rb @@ -35,8 +35,12 @@ module ActionView end end - def instrument(name, options={}) - ActiveSupport::Notifications.instrument("render_#{name}.action_view", options){ yield } + def instrument(name, **options) + options[:identifier] ||= (@template && @template.identifier) || @path + + ActiveSupport::Notifications.instrument("render_#{name}.action_view", options) do |payload| + yield payload + end end def prepend_formats(formats) diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index a9bd257e76..514804b08e 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -294,7 +294,7 @@ module ActionView def render(context, options, block) setup(context, options, block) - identifier = (@template = find_partial) ? @template.identifier : @path + @template = find_partial @lookup_context.rendered_format ||= begin if @template && @template.formats.present? @@ -305,11 +305,9 @@ module ActionView end if @collection - instrument(:collection, :identifier => identifier || "collection", :count => @collection.size) do - render_collection - end + render_collection else - instrument(:partial, :identifier => identifier) do + instrument(:partial) do render_partial end end @@ -318,15 +316,17 @@ module ActionView private def render_collection - return nil if @collection.blank? + instrument(:collection, count: @collection.size) do |payload| + return nil if @collection.blank? - if @options.key?(:spacer_template) - spacer = find_template(@options[:spacer_template], @locals.keys).render(@view, @locals) - end + if @options.key?(:spacer_template) + spacer = find_template(@options[:spacer_template], @locals.keys).render(@view, @locals) + end - cache_collection_render do - @template ? collection_with_template : collection_without_template - end.join(spacer).html_safe + cache_collection_render(payload) do + @template ? collection_with_template : collection_without_template + end.join(spacer).html_safe + end end def render_partial @@ -428,8 +428,6 @@ 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 @@ -438,11 +436,6 @@ 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 4860f00243..f7deba94ce 100644 --- a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb +++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb @@ -1,5 +1,3 @@ -require 'active_support/core_ext/object/try' - module ActionView module CollectionCaching # :nodoc: extend ActiveSupport::Concern @@ -11,40 +9,25 @@ module ActionView end private - def cache_collection_render - return yield unless cache_collection? + def cache_collection_render(instrumentation_payload) + return yield unless @options[:cached] keyed_collection = collection_by_cache_keys - cached_partials = collection_cache.read_multi(*keyed_collection.keys) + cached_partials = collection_cache.read_multi(*keyed_collection.keys) + instrumentation_payload[:cache_hits] = cached_partials.size @collection = keyed_collection.reject { |key, _| cached_partials.key?(key) }.values rendered_partials = @collection.empty? ? [] : yield index = 0 - keyed_collection.map do |cache_key, _| - cached_partials.fetch(cache_key) do - rendered_partials[index].tap { index += 1 } - end + fetch_or_cache_partial(cached_partials, order_by: keyed_collection.each_key) do + rendered_partials[index].tap { index += 1 } end end - def cache_collection? - @options.fetch(:cache, automatic_cache_eligible?) - end - - def automatic_cache_eligible? - @template && @template.eligible_for_collection_caching?(as: @options[:as]) - end - - def callable_cache_key? - @options[:cache].respond_to?(:call) - end - def collection_by_cache_keys - seed = callable_cache_key? ? @options[:cache] : ->(i) { i } - @collection.each_with_object({}) do |item, hash| - hash[expanded_cache_key(seed.call(item))] = item + hash[expanded_cache_key(item)] = item end end @@ -53,10 +36,13 @@ module ActionView key.frozen? ? key.dup : key # #read_multi & #write may require mutability, Dalli 2.6.0. 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]) + def fetch_or_cache_partial(cached_partials, order_by:) + order_by.map do |cache_key| + cached_partials.fetch(cache_key) do + yield.tap do |rendered_partial| + collection_cache.write(cache_key, rendered_partial) + end + end end end end diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 15fc2b71a3..169ee55fdc 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -130,7 +130,6 @@ module ActionView @source = source @identifier = identifier @handler = handler - @cache_name = extract_resource_cache_name @compiled = false @original_encoding = nil @locals = details[:locals] || [] @@ -166,10 +165,6 @@ module ActionView @type ||= Types[@formats.first] if @formats.first end - def eligible_for_collection_caching?(as: nil) - @cache_name == (as || inferred_cache_name).to_s - end - # Receives a view object and return a template similar to self by using @virtual_path. # # This method is useful if you have a template object but it does not contain its source @@ -355,23 +350,5 @@ module ActionView ActiveSupport::Notifications.instrument("#{action}.action_view".freeze, payload, &block) end end - - EXPLICIT_COLLECTION = /# Template Collection: (?<resource_name>\w+)/ - - def extract_resource_cache_name - if match = @source.match(EXPLICIT_COLLECTION) || resource_cache_call_match - match[:resource_name] - end - end - - def resource_cache_call_match - if @handler.respond_to?(:resource_cache_call_pattern) - @source.match(@handler.resource_cache_call_pattern) - end - end - - def inferred_cache_name - @inferred_cache_name ||= @virtual_path.split('/'.freeze).last.sub('_'.freeze, ''.freeze) - end end end diff --git a/actionview/lib/action_view/template/handlers/erb.rb b/actionview/lib/action_view/template/handlers/erb.rb index 1f8459c24b..85a100ed4c 100644 --- a/actionview/lib/action_view/template/handlers/erb.rb +++ b/actionview/lib/action_view/template/handlers/erb.rb @@ -123,31 +123,6 @@ module ActionView ).src end - # Returns Regexp to extract a cached resource's name from a cache call at the - # first line of a template. - # The extracted cache name is captured as :resource_name. - # - # <% cache notification do %> # => notification - # - # The pattern should support templates with a beginning comment: - # - # <%# Still extractable even though there's a comment %> - # <% cache notification do %> # => notification - # - # But fail to extract a name if a resource association is cached. - # - # <% cache notification.event do %> # => nil - def resource_cache_call_pattern - /\A - (?:<%\#.*%>)* # optional initial comment - \s* # followed by optional spaces or newlines - <%\s*cache[\(\s] # followed by an ERB call to cache - \s* # followed by optional spaces or newlines - (?<resource_name>\w+) # capture the cache call argument as :resource_name - [\s\)] # followed by a space or close paren - /xm - end - private def valid_encoding(string, encoding) diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index 9ff5f7126b..a9a6de250c 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -26,6 +26,7 @@ class TemplateDigestorTest < ActionView::TestCase @cwd = Dir.pwd @tmp_dir = Dir.mktmpdir + ActionView::LookupContext::DetailsKey.clear FileUtils.cp_r FixtureFinder::FIXTURES_DIR, @tmp_dir Dir.chdir @tmp_dir end diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 4776c18b0b..93a0701dcc 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -86,7 +86,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered test\/_customer.erb/, @logger.logged(:info).last) + assert_match(/Rendered collection of test\/_customer.erb \[2 times\]/, @logger.logged(:info).last) end end @@ -96,7 +96,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered customers\/_customer\.html\.erb/, @logger.logged(:info).last) + assert_match(/Rendered collection of customers\/_customer\.html\.erb \[2 times\]/, @logger.logged(:info).last) end end @@ -106,7 +106,21 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered collection/, @logger.logged(:info).last) + assert_match(/Rendered collection of templates/, @logger.logged(:info).last) + end + end + + def test_render_collection_with_cached_set + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + def @view.view_cache_dependencies; []; end + def @view.fragment_cache_key(*); 'ahoy `controller` dependency'; end + + @view.render(partial: 'customers/customer', collection: [ Customer.new('david'), Customer.new('mary') ], cached: true, + locals: { greeting: 'hi' }) + wait + + assert_equal 1, @logger.logged(:info).size + assert_match(/Rendered collection of customers\/_customer\.html\.erb \[0 \/ 2 cache hits\]/, @logger.logged(:info).last) end end end diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index bf811abdd0..3561114d90 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -628,56 +628,59 @@ class LazyViewRenderTest < ActiveSupport::TestCase end end -class CachedCollectionViewRenderTest < CachedViewRenderTest +class CachedCollectionViewRenderTest < ActiveSupport::TestCase class CachedCustomer < Customer; end - teardown do - ActionView::PartialRenderer.collection_cache.clear - end + include RenderTestCases - test "with custom key" do - customer = Customer.new("david") - key = cache_key([customer, 'key'], "test/_customer") + # Ensure view path cache is primed + setup do + view_paths = ActionController::Base.view_paths + assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class - ActionView::PartialRenderer.collection_cache.write(key, 'Hello') + ActionView::PartialRenderer.collection_cache = ActiveSupport::Cache::MemoryStore.new - assert_equal "Hello", - @view.render(partial: "test/customer", collection: [customer], cache: ->(item) { [item, 'key'] }) + setup_view(view_paths) end - test "with caching with custom key and rendering with different key" do - customer = Customer.new("david") - key = cache_key([customer, 'key'], "test/_customer") + teardown do + GC.start + I18n.reload! + end - ActionView::PartialRenderer.collection_cache.write(key, 'Hello') + test "collection caching does not cache by default" do + customer = Customer.new("david", 1) + key = cache_key(customer, "test/_customer") - assert_equal "Hello: david", - @view.render(partial: "test/customer", collection: [customer], cache: ->(item) { [item, 'another_key'] }) + ActionView::PartialRenderer.collection_cache.write(key, 'Cached') + + assert_not_equal "Cached", + @view.render(partial: "test/customer", collection: [customer]) end - test "automatic caching with inferred cache name" do - customer = CachedCustomer.new("david") - key = cache_key(customer, "test/_cached_customer") + test "collection caching with partial that doesn't use fragment caching" do + customer = Customer.new("david", 1) + key = cache_key(customer, "test/_customer") ActionView::PartialRenderer.collection_cache.write(key, 'Cached') assert_equal "Cached", - @view.render(partial: "test/cached_customer", collection: [customer]) + @view.render(partial: "test/customer", collection: [customer], cached: true) end - test "automatic caching with as name" do - customer = CachedCustomer.new("david") - key = cache_key(customer, "test/_cached_customer_as") + test "collection caching with cached true" do + customer = CachedCustomer.new("david", 1) + key = cache_key(customer, "test/_cached_customer") ActionView::PartialRenderer.collection_cache.write(key, 'Cached') assert_equal "Cached", - @view.render(partial: "test/cached_customer_as", collection: [customer], as: :buyer) + @view.render(partial: "test/cached_customer", collection: [customer], cached: true) end private - def cache_key(names, virtual_path) + def cache_key(*names, virtual_path) digest = ActionView::Digestor.digest name: virtual_path, finder: @view.lookup_context, dependencies: [] - @view.fragment_cache_key([ *Array(names), digest ]) + @view.fragment_cache_key([ *names, digest ]) end end diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index 921011b073..533c1c3219 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -192,38 +192,6 @@ class TestERBTemplate < ActiveSupport::TestCase assert_match(/\xFC/, e.message) end - def test_not_eligible_for_collection_caching_without_cache_call - [ - "<%= 'Hello' %>", - "<% cache_customer = 42 %>", - "<% cache customer.name do %><% end %>", - "<% my_cache customer do %><% end %>" - ].each do |body| - template = new_template(body, virtual_path: "test/foo/_customer") - assert_not template.eligible_for_collection_caching?, "Template #{body.inspect} should not be eligible for collection caching" - end - end - - def test_eligible_for_collection_caching_with_cache_call_or_explicit - [ - "<% cache customer do %><% end %>", - "<% cache(customer) do %><% end %>", - "<% cache( customer) do %><% end %>", - "<% cache( customer ) do %><% end %>", - "<%cache customer do %><% end %>", - "<% cache customer do %><% end %>", - " <% cache customer do %>\n<% end %>\n", - "<%# comment %><% cache customer do %><% end %>", - "<%# comment %>\n<% cache customer do %><% end %>", - "<%# comment\n line 2\n line 3 %>\n<% cache customer do %><% end %>", - "<%# comment 1 %>\n<%# comment 2 %>\n<% cache customer do %><% end %>", - "<%# comment 1 %>\n<%# Template Collection: customer %>\n<% my_cache customer do %><% end %>" - ].each do |body| - template = new_template(body, virtual_path: "test/foo/_customer") - assert template.eligible_for_collection_caching?, "Template #{body.inspect} should be eligible for collection caching" - end - end - def with_external_encoding(encoding) old = Encoding.default_external Encoding::Converter.new old, encoding if old != encoding diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index ea69e7549e..36834bbd36 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -327,7 +327,7 @@ module ActiveModel # # => {:base=>[{error: :name_or_email_blank}]} def add(attribute, message = :invalid, options = {}) message = message.call if message.respond_to?(:call) - detail = normalize_detail(attribute, message, options) + detail = normalize_detail(message, options) message = normalize_message(attribute, message, options) if exception = options[:strict] exception = ActiveModel::StrictValidationFailed if exception == true @@ -502,7 +502,7 @@ module ActiveModel end end - def normalize_detail(attribute, message, options) + def normalize_detail(message, options) { error: message }.merge(options.except(*CALLBACKS_OPTIONS + MESSAGE_OPTIONS)) end end diff --git a/activemodel/lib/active_model/validator.rb b/activemodel/lib/active_model/validator.rb index 21339b628e..109bf038b0 100644 --- a/activemodel/lib/active_model/validator.rb +++ b/activemodel/lib/active_model/validator.rb @@ -163,17 +163,6 @@ module ActiveModel # +ArgumentError+ when invalid options are supplied. def check_validity! end - - def should_validate?(record) # :nodoc: - !record.persisted? || record.changed? || record.marked_for_destruction? - end - - # Always validate the record if the attribute is a virtual attribute. - # We have no way of knowing that the record was changed if the attribute - # does not exist in the database. - def unknown_attribute?(record, attribute) # :nodoc: - !record.attributes.include?(attribute.to_s) - end end # +BlockValidator+ is a special +EachValidator+ which receives a block on initialization diff --git a/activemodel/test/cases/validations/format_validation_test.rb b/activemodel/test/cases/validations/format_validation_test.rb index 86bbbe6ebe..ea4c2ee7df 100644 --- a/activemodel/test/cases/validations/format_validation_test.rb +++ b/activemodel/test/cases/validations/format_validation_test.rb @@ -73,7 +73,7 @@ class PresenceValidationTest < ActiveModel::TestCase end def test_validate_format_of_with_multiline_regexp_and_option - assert_nothing_raised(ArgumentError) do + assert_nothing_raised do Topic.validates_format_of(:title, with: /^Valid Title$/, multiline: true) end end diff --git a/activemodel/test/cases/validations/inclusion_validation_test.rb b/activemodel/test/cases/validations/inclusion_validation_test.rb index 55d1fb4dcb..6cb96343fa 100644 --- a/activemodel/test/cases/validations/inclusion_validation_test.rb +++ b/activemodel/test/cases/validations/inclusion_validation_test.rb @@ -58,9 +58,9 @@ class InclusionValidationTest < ActiveModel::TestCase assert_raise(ArgumentError) { Topic.validates_inclusion_of(:title, in: nil) } assert_raise(ArgumentError) { Topic.validates_inclusion_of(:title, in: 0) } - assert_nothing_raised(ArgumentError) { Topic.validates_inclusion_of(:title, in: "hi!") } - assert_nothing_raised(ArgumentError) { Topic.validates_inclusion_of(:title, in: {}) } - assert_nothing_raised(ArgumentError) { Topic.validates_inclusion_of(:title, in: []) } + assert_nothing_raised { Topic.validates_inclusion_of(:title, in: "hi!") } + assert_nothing_raised { Topic.validates_inclusion_of(:title, in: {}) } + assert_nothing_raised { Topic.validates_inclusion_of(:title, in: []) } end def test_validates_inclusion_of_with_allow_nil diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 641e9195c6..c18403865f 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* Ensure that mutations of the array returned from `ActiveRecord::Relation#to_a` + do not affect the original relation, by returning a duplicate array each time. + + This brings the behavior in line with `CollectionProxy#to_a`, which was + already more careful. + + *Matthew Draper* + * Fixed `where` for polymorphic associations when passed an array containing different types. Fixes #17011. diff --git a/activerecord/lib/active_record/association_relation.rb b/activerecord/lib/active_record/association_relation.rb index ee0bb8fafe..c18e88e4cf 100644 --- a/activerecord/lib/active_record/association_relation.rb +++ b/activerecord/lib/active_record/association_relation.rb @@ -10,7 +10,7 @@ module ActiveRecord end def ==(other) - other == to_a + other == records end def build(*args, &block) diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 2a9627a474..b9aed05135 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -979,6 +979,10 @@ module ActiveRecord end alias_method :to_a, :to_ary + def records # :nodoc: + load_target + end + # Adds one or more +records+ to the collection by setting their foreign keys # to the association's primary key. Returns +self+, so several appends may be # chained together. diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index c7cc48ba16..f913f0852a 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -45,7 +45,7 @@ module ActiveRecord end def get_records - return scope.limit(1).to_a if skip_statement_cache? + return scope.limit(1).records if skip_statement_cache? conn = klass.connection sc = reflection.association_scope_cache(conn, owner) do 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 bb5119d64e..aa5ae15285 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -30,7 +30,7 @@ module ActiveRecord 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) && preparable.nil? + if !prepared_statements || (arel.is_a?(String) && preparable.nil?) preparable = false else preparable = visitor.preparable diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 23b33a3555..57d8867bb4 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -134,8 +134,6 @@ module ActiveRecord ActiveRecord::Result.new(result.fields, result.to_a) end - alias exec_without_stmt exec_query - def exec_insert(sql, name, binds, pk = nil, sequence_name = nil) execute to_sql(sql, binds), name end diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 45e35a4f71..09d55adcd7 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -102,7 +102,7 @@ module ActiveRecord module Legacy include FourTwoShared - def run(*) + def migrate(*) ActiveSupport::Deprecation.warn \ "Directly inheriting from ActiveRecord::Migration is deprecated. " \ "Please specify the Rails release the migration was written for:\n" \ diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 0d5a8e6f25..ae78ceee01 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -195,15 +195,23 @@ module ActiveRecord # Nested attributes for an associated collection can also be passed in # the form of a hash of hashes instead of an array of hashes: # - # Member.create(name: 'joe', - # posts_attributes: { first: { title: 'Foo' }, - # second: { title: 'Bar' } }) + # Member.create( + # name: 'joe', + # posts_attributes: { + # first: { title: 'Foo' }, + # second: { title: 'Bar' } + # } + # ) # # has the same effect as # - # Member.create(name: 'joe', - # posts_attributes: [ { title: 'Foo' }, - # { title: 'Bar' } ]) + # Member.create( + # name: 'joe', + # posts_attributes: [ + # { title: 'Foo' }, + # { title: 'Bar' } + # ] + # ) # # The keys of the hash which is the value for +:posts_attributes+ are # ignored in this case. diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 0b500346bc..1ab4e0404f 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -1,7 +1,7 @@ module ActiveRecord module NullRelation # :nodoc: def exec_queries - @records = [] + @records = [].freeze end def pluck(*column_names) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 7e842668c6..09afdc6c69 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -253,17 +253,21 @@ module ActiveRecord # Converts relation objects to Array. def to_a + records.dup + end + + def records # :nodoc: load @records end # Serializes the relation objects Array. def encode_with(coder) - coder.represent_seq(nil, to_a) + coder.represent_seq(nil, records) end def as_json(options = nil) #:nodoc: - to_a.as_json(options) + records.as_json(options) end # Returns size of the records. @@ -298,13 +302,13 @@ module ActiveRecord # Returns true if there is exactly one record. def one? return super if block_given? - limit_value ? to_a.one? : size == 1 + limit_value ? records.one? : size == 1 end # Returns true if there is more than one record. def many? return super if block_given? - limit_value ? to_a.many? : size > 1 + limit_value ? records.many? : size > 1 end # Returns a cache key that can be used to identify the records fetched by @@ -418,7 +422,7 @@ module ActiveRecord if id.is_a?(Array) id.map.with_index { |one_id, idx| update(one_id, attributes[idx]) } elsif id == :all - to_a.each { |record| record.update(attributes) } + records.each { |record| record.update(attributes) } else if ActiveRecord::Base === id id = id.id @@ -457,7 +461,7 @@ module ActiveRecord MESSAGE where(conditions).destroy_all else - to_a.each(&:destroy).tap { reset } + records.each(&:destroy).tap { reset } end end @@ -587,7 +591,7 @@ module ActiveRecord def reset @last = @to_sql = @order_clause = @scope_for_create = @arel = @loaded = nil @should_eager_load = @join_dependency = nil - @records = [] + @records = [].freeze @offsets = {} self end @@ -654,21 +658,21 @@ module ActiveRecord def ==(other) case other when Associations::CollectionProxy, AssociationRelation - self == other.to_a + self == other.records when Relation other.to_sql == to_sql when Array - to_a == other + records == other end end def pretty_print(q) - q.pp(self.to_a) + q.pp(self.records) end # Returns true if relation is blank. def blank? - to_a.blank? + records.blank? end def values @@ -676,7 +680,7 @@ module ActiveRecord end def inspect - entries = to_a.take([limit_value, 11].compact.min).map!(&:inspect) + entries = records.take([limit_value, 11].compact.min).map!(&:inspect) entries[10] = '...' if entries.size == 11 "#<#{self.class.name} [#{entries.join(', ')}]>" @@ -685,14 +689,14 @@ module ActiveRecord protected def load_records(records) - @records = records + @records = records.freeze @loaded = true end private def exec_queries - @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bound_attributes) + @records = eager_loading? ? find_with_associations.freeze : @klass.find_by_sql(arel, bound_attributes).freeze preload = preload_values preload += includes_values unless eager_loading? diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index de005e2810..243ef0eae9 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -187,7 +187,7 @@ module ActiveRecord loop do if load - records = batch_relation.to_a + records = batch_relation.records ids = records.map(&:id) yielded_relation = self.where(primary_key => ids) yielded_relation.load_records(records) diff --git a/activerecord/lib/active_record/relation/batches/batch_enumerator.rb b/activerecord/lib/active_record/relation/batches/batch_enumerator.rb index c6e39814dd..13393dc605 100644 --- a/activerecord/lib/active_record/relation/batches/batch_enumerator.rb +++ b/activerecord/lib/active_record/relation/batches/batch_enumerator.rb @@ -35,7 +35,7 @@ module ActiveRecord return to_enum(:each_record) unless block_given? @relation.to_enum(:in_batches, of: @of, start: @start, finish: @finish, load: true).each do |relation| - relation.to_a.each { |record| yield record } + relation.records.each { |record| yield record } end end diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 2b1ac42395..f2578f5f96 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -38,7 +38,7 @@ module ActiveRecord delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :join, :[], :&, :|, :+, :-, :sample, :reverse, :compact, :in_groups, :in_groups_of, - :shuffle, :split, to: :to_a + :shuffle, :split, to: :records delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :columns_hash, :to => :klass diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 16563515f6..0037398554 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -506,7 +506,7 @@ module ActiveRecord def find_some_ordered(ids) ids = ids.slice(offset_value || 0, limit_value || ids.size) || [] - result = except(:limit, :offset).where(primary_key => ids).to_a + result = except(:limit, :offset).where(primary_key => ids).records if result.size == ids.size pk_type = @klass.type_for_attribute(primary_key) @@ -522,7 +522,7 @@ module ActiveRecord if loaded? @records.first else - @take ||= limit(1).to_a.first + @take ||= limit(1).records.first end end @@ -573,7 +573,7 @@ module ActiveRecord end def find_last(limit) - limit ? to_a.last(limit) : to_a.last + limit ? records.last(limit) : records.last end end end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 67d7f83cb4..d5c18a2a4a 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -29,7 +29,7 @@ module ActiveRecord # This is mainly intended for sharing common conditions between multiple associations. def merge(other) if other.is_a?(Array) - to_a & other + records & other elsif other spawn.merge!(other) else diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 8f52e9068a..7dc41fa98c 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -116,7 +116,11 @@ module ActiveRecord end def create_all + old_pool = ActiveRecord::Base.connection_handler.retrieve_connection_pool(ActiveRecord::Base) each_local_configuration { |configuration| create configuration } + if old_pool + ActiveRecord::Base.connection_handler.establish_connection(ActiveRecord::Base, old_pool.spec) + end end def create_current(environment = env) diff --git a/activerecord/lib/active_record/validations/absence.rb b/activerecord/lib/active_record/validations/absence.rb index 376d743c92..641d041f3d 100644 --- a/activerecord/lib/active_record/validations/absence.rb +++ b/activerecord/lib/active_record/validations/absence.rb @@ -2,7 +2,6 @@ module ActiveRecord module Validations class AbsenceValidator < ActiveModel::Validations::AbsenceValidator # :nodoc: def validate_each(record, attribute, association_or_value) - return unless should_validate?(record) || unknown_attribute?(record, attribute) if record.class._reflect_on_association(attribute) association_or_value = Array.wrap(association_or_value).reject(&:marked_for_destruction?) end diff --git a/activerecord/lib/active_record/validations/length.rb b/activerecord/lib/active_record/validations/length.rb index fe34e4875c..0e0cebce4a 100644 --- a/activerecord/lib/active_record/validations/length.rb +++ b/activerecord/lib/active_record/validations/length.rb @@ -2,23 +2,11 @@ module ActiveRecord module Validations class LengthValidator < ActiveModel::Validations::LengthValidator # :nodoc: def validate_each(record, attribute, association_or_value) - return unless should_validate?(record) || unknown_attribute?(record, attribute) || associations_are_dirty?(record) if association_or_value.respond_to?(:loaded?) && association_or_value.loaded? association_or_value = association_or_value.target.reject(&:marked_for_destruction?) end super end - - def associations_are_dirty?(record) - attributes.any? do |attribute| - value = record.read_attribute_for_validation(attribute) - if value.respond_to?(:loaded?) && value.loaded? - value.target.any?(&:marked_for_destruction?) - else - false - end - end - end end module ClassMethods diff --git a/activerecord/lib/active_record/validations/presence.rb b/activerecord/lib/active_record/validations/presence.rb index e34d2d70ab..ad82ea66c4 100644 --- a/activerecord/lib/active_record/validations/presence.rb +++ b/activerecord/lib/active_record/validations/presence.rb @@ -2,7 +2,6 @@ module ActiveRecord module Validations class PresenceValidator < ActiveModel::Validations::PresenceValidator # :nodoc: def validate_each(record, attribute, association_or_value) - return unless should_validate?(record) || unknown_attribute?(record, attribute) if record.class._reflect_on_association(attribute) association_or_value = Array.wrap(association_or_value).reject(&:marked_for_destruction?) end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index f0aa4521b5..13053beb78 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -11,13 +11,12 @@ module ActiveRecord end def validate_each(record, attribute, value) - return unless should_validate?(record) finder_class = find_finder_class_for(record) table = finder_class.arel_table value = map_enum_attribute(finder_class, attribute, value) relation = build_relation(finder_class, table, attribute, value) - if record.persisted? && finder_class.primary_key.to_s != attribute.to_s + if record.persisted? if finder_class.primary_key relation = relation.where.not(finder_class.primary_key => record.id_was) else @@ -57,14 +56,13 @@ module ActiveRecord value = value.attributes[reflection.klass.primary_key] unless value.nil? end - attribute_name = attribute.to_s - # the attribute may be an aliased attribute - if klass.attribute_aliases[attribute_name] - attribute = klass.attribute_aliases[attribute_name] - attribute_name = attribute.to_s + if klass.attribute_alias?(attribute) + attribute = klass.attribute_alias(attribute) end + attribute_name = attribute.to_s + column = klass.columns_hash[attribute_name] cast_type = klass.type_for_attribute(attribute_name) value = cast_type.serialize(value) @@ -82,7 +80,7 @@ module ActiveRecord if value.nil? klass.unscoped.where(comparison) else - bind = Relation::QueryAttribute.new(attribute.to_s, value, Type::Value.new) + bind = Relation::QueryAttribute.new(attribute_name, value, Type::Value.new) klass.unscoped.where(comparison, bind) end rescue RangeError diff --git a/activerecord/lib/rails/generators/active_record/model/model_generator.rb b/activerecord/lib/rails/generators/active_record/model/model_generator.rb index 7395839fca..179a4dc91e 100644 --- a/activerecord/lib/rails/generators/active_record/model/model_generator.rb +++ b/activerecord/lib/rails/generators/active_record/model/model_generator.rb @@ -22,11 +22,13 @@ module ActiveRecord def create_model_file template 'model.rb', File.join('app/models', class_path, "#{file_name}.rb") + generate_application_record end def create_module_file return if regular_class_path.empty? template 'module.rb', File.join('app/models', "#{class_path.join('/')}.rb") if behavior == :invoke + generate_application_record end hook_for :test_framework @@ -37,6 +39,13 @@ module ActiveRecord attributes.select { |a| !a.reference? && a.has_index? } end + # FIXME: Change this file to a symlink once RubyGems 2.5.0 is required. + def generate_application_record + if self.behavior == :invoke && !File.exist?('app/models/application_record.rb') + template 'application_record.rb', 'app/models/application_record.rb' + end + end + # Used by the migration template to determine the parent name of the model def parent_class_name options[:parent] || determine_default_parent_class diff --git a/activerecord/lib/rails/generators/active_record/model/templates/application_record.rb b/activerecord/lib/rails/generators/active_record/model/templates/application_record.rb new file mode 100644 index 0000000000..10a4cba84d --- /dev/null +++ b/activerecord/lib/rails/generators/active_record/model/templates/application_record.rb @@ -0,0 +1,3 @@ +class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true +end diff --git a/activerecord/test/cases/adapters/postgresql/prepared_statements_test.rb b/activerecord/test/cases/adapters/postgresql/prepared_statements_test.rb new file mode 100644 index 0000000000..f1519db48b --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/prepared_statements_test.rb @@ -0,0 +1,22 @@ +require "cases/helper" +require "models/developer" + +class PreparedStatementsTest < ActiveRecord::PostgreSQLTestCase + fixtures :developers + + def setup + @default_prepared_statements = Developer.connection_config[:prepared_statements] + Developer.connection_config[:prepared_statements] = false + end + + def teardown + Developer.connection_config[:prepared_statements] = @default_prepared_statements + end + + def nothing_raised_with_falsy_prepared_statements + assert_nothing_raised do + Developer.where(id: 1) + end + end + +end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 874d53c51f..ac478cbb01 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1216,7 +1216,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_join_eager_with_empty_order_should_generate_valid_sql - assert_nothing_raised(ActiveRecord::StatementInvalid) do + assert_nothing_raised do Post.includes(:comments).order("").where(:comments => {:body => "Thank you for the welcome"}).first end end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 9096cbc0ab..1bbca84bb2 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -938,7 +938,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase end def test_with_symbol_class_name - assert_nothing_raised NoMethodError do + assert_nothing_raised do DeveloperWithSymbolClassName.new end end diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 57d1c8feda..c9743e80d3 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -130,15 +130,15 @@ end class InverseAssociationTests < ActiveRecord::TestCase def test_should_allow_for_inverse_of_options_in_associations - assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on has_many') do + assert_nothing_raised do Class.new(ActiveRecord::Base).has_many(:wheels, :inverse_of => :car) end - assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on has_one') do + assert_nothing_raised do Class.new(ActiveRecord::Base).has_one(:engine, :inverse_of => :car) end - assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on belongs_to') do + assert_nothing_raised do Class.new(ActiveRecord::Base).belongs_to(:car, :inverse_of => :driver) end end @@ -666,7 +666,7 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase def test_trying_to_access_inverses_that_dont_exist_shouldnt_raise_an_error # Ideally this would, if only for symmetry's sake with other association types - assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.first.horrible_polymorphic_man } + assert_nothing_raised { Face.first.horrible_polymorphic_man } end def test_trying_to_set_polymorphic_inverses_that_dont_exist_at_all_should_raise_an_error @@ -676,7 +676,7 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase def test_trying_to_set_polymorphic_inverses_that_dont_exist_on_the_instance_being_set_should_raise_an_error # passes because Man does have the correct inverse_of - assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.first.polymorphic_man = Man.first } + assert_nothing_raised { Face.first.polymorphic_man = Man.first } # fails because Interest does have the correct inverse_of assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.first.polymorphic_man = Interest.first } end @@ -688,7 +688,7 @@ class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase fixtures :men, :interests, :zines def test_that_we_can_load_associations_that_have_the_same_reciprocal_name_from_different_models - assert_nothing_raised(ActiveRecord::AssociationTypeMismatch) do + assert_nothing_raised do i = Interest.first i.zine i.man @@ -696,7 +696,7 @@ class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase end def test_that_we_can_create_associations_that_have_the_same_reciprocal_name_from_different_models - assert_nothing_raised(ActiveRecord::AssociationTypeMismatch) do + assert_nothing_raised do i = Interest.first i.build_zine(:title => 'Get Some in Winter! 2008') i.build_man(:name => 'Gordon') diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index f6dddaf5b4..c850875619 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -88,7 +88,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_polymorphic_has_many_going_through_join_model_with_custom_select_and_joins assert_equal tags(:general), tag = posts(:welcome).tags.add_joins_and_select.first - assert_nothing_raised(NoMethodError) { tag.author_id } + assert_nothing_raised { tag.author_id } end def test_polymorphic_has_many_going_through_join_model_with_custom_foreign_key diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index c922a8d1c2..8f2682c781 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -124,7 +124,7 @@ class CalculationsTest < ActiveRecord::TestCase end def test_should_generate_valid_sql_with_joins_and_group - assert_nothing_raised ActiveRecord::StatementInvalid do + assert_nothing_raised do AuditLog.joins(:developer).group(:id).count end end @@ -742,7 +742,7 @@ class CalculationsTest < ActiveRecord::TestCase end def test_should_reference_correct_aliases_while_joining_tables_of_has_many_through_association - assert_nothing_raised ActiveRecord::StatementInvalid do + assert_nothing_raised do developer = Developer.create!(name: 'developer') developer.ratings.includes(comment: :post).where(posts: { id: 1 }).count end diff --git a/activerecord/test/cases/counter_cache_test.rb b/activerecord/test/cases/counter_cache_test.rb index 922cb59280..66b4c3f1ff 100644 --- a/activerecord/test/cases/counter_cache_test.rb +++ b/activerecord/test/cases/counter_cache_test.rb @@ -151,7 +151,7 @@ class CounterCacheTest < ActiveRecord::TestCase test "reset the right counter if two have the same foreign key" do michael = people(:michael) - assert_nothing_raised(ActiveRecord::StatementInvalid) do + assert_nothing_raised do Person.reset_counters(michael.id, :friends_too) end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 5b41630cd8..3e31874455 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -43,7 +43,7 @@ class FinderTest < ActiveRecord::TestCase end assert_equal "should happen", exception.message - assert_nothing_raised(RuntimeError) do + assert_nothing_raised do Topic.all.find(-> { raise "should not happen" }) { |e| e.title == topics(:first).title } end end @@ -540,7 +540,7 @@ class FinderTest < ActiveRecord::TestCase assert_deprecated do Topic.order("coalesce(author_name, title)").last end - end + end def test_last_on_relation_with_limit_and_offset post = posts('sti_comments') @@ -1086,7 +1086,7 @@ class FinderTest < ActiveRecord::TestCase end def test_finder_with_offset_string - assert_nothing_raised(ActiveRecord::StatementInvalid) { Topic.offset("3").to_a } + assert_nothing_raised { Topic.offset("3").to_a } end test "find_by with hash conditions returns the first matching record" do diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 6d5b6243db..60ca90464d 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -21,7 +21,7 @@ module ActiveRecord teardown do connection.drop_table :testings rescue nil ActiveRecord::Migration.verbose = @verbose_was - ActiveRecord::SchemaMigration.delete_all + ActiveRecord::SchemaMigration.delete_all rescue nil end def test_migration_doesnt_remove_named_index @@ -101,6 +101,18 @@ module ActiveRecord assert connection.columns(:testings).find { |c| c.name == 'created_at' }.null assert connection.columns(:testings).find { |c| c.name == 'updated_at' }.null end + + def test_legacy_migrations_get_deprecation_warning_when_run + migration = Class.new(ActiveRecord::Migration) { + def up + add_column :testings, :baz, :string + end + } + + assert_deprecated do + migration.migrate :up + end + end end end end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index bae0467e72..c88e8d4a8b 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -586,8 +586,7 @@ class MigrationTest < ActiveRecord::TestCase end if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) - def test_out_of_range_limit_should_raise - Person.connection.drop_table :test_limits rescue nil + def test_out_of_range_integer_limit_should_raise e = assert_raise(ActiveRecord::ActiveRecordError, "integer limit didn't raise") do Person.connection.create_table :test_integer_limits, :force => true do |t| t.column :bigone, :integer, :limit => 10 @@ -595,16 +594,22 @@ class MigrationTest < ActiveRecord::TestCase end assert_match(/No integer type has byte size 10/, e.message) + ensure + Person.connection.drop_table :test_integer_limits, if_exists: true + end + end - unless current_adapter?(:PostgreSQLAdapter) - assert_raise(ActiveRecord::ActiveRecordError, "text limit didn't raise") do - Person.connection.create_table :test_text_limits, :force => true do |t| - t.column :bigtext, :text, :limit => 0xfffffffff - end + if current_adapter?(:Mysql2Adapter) + def test_out_of_range_text_limit_should_raise + e = assert_raise(ActiveRecord::ActiveRecordError, "text limit didn't raise") do + Person.connection.create_table :test_text_limits, force: true do |t| + t.text :bigtext, limit: 0xfffffffff end end - Person.connection.drop_table :test_limits rescue nil + assert_match(/No text type has byte length #{0xfffffffff}/, e.message) + ensure + Person.connection.drop_table :test_text_limits, if_exists: true end end @@ -738,7 +743,7 @@ class ExplicitlyNamedIndexMigrationTest < ActiveRecord::TestCase t.integer :value end - assert_nothing_raised ArgumentError do + assert_nothing_raised do connection.add_index :values, :value, name: 'a_different_name' connection.remove_index :values, column: :value, name: 'a_different_name' end diff --git a/activerecord/test/cases/modules_test.rb b/activerecord/test/cases/modules_test.rb index 7f31325f47..486bcc22df 100644 --- a/activerecord/test/cases/modules_test.rb +++ b/activerecord/test/cases/modules_test.rb @@ -63,7 +63,7 @@ class ModulesTest < ActiveRecord::TestCase def test_assign_ids firm = MyApplication::Business::Firm.first - assert_nothing_raised NameError, "Should be able to resolve all class constants via reflection" do + assert_nothing_raised do firm.client_ids = [MyApplication::Business::Client.first.id] end end @@ -72,7 +72,7 @@ class ModulesTest < ActiveRecord::TestCase def test_eager_loading_in_modules clients = [] - assert_nothing_raised NameError, "Should be able to resolve all class constants via reflection" do + assert_nothing_raised do clients << MyApplication::Business::Client.references(:accounts).merge!(:includes => {:firm => :account}, :where => 'accounts.id IS NOT NULL').find(3) clients << MyApplication::Business::Client.includes(:firm => :account).find(3) end diff --git a/activerecord/test/cases/multiple_db_test.rb b/activerecord/test/cases/multiple_db_test.rb index 39cdcf5403..af4183a601 100644 --- a/activerecord/test/cases/multiple_db_test.rb +++ b/activerecord/test/cases/multiple_db_test.rb @@ -104,7 +104,7 @@ class MultipleDbTest < ActiveRecord::TestCase def test_associations_should_work_when_model_has_no_connection begin ActiveRecord::Base.remove_connection - assert_nothing_raised ActiveRecord::ConnectionNotEstablished do + assert_nothing_raised do College.first.courses.first end ensure diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 38a4072114..11fb164d50 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -76,7 +76,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase pirate.update(ship_attributes: { '_destroy' => true, :id => ship.id }) - assert_nothing_raised(ActiveRecord::RecordNotFound) { pirate.ship.reload } + assert_nothing_raised { pirate.ship.reload } end def test_a_model_should_respond_to_underscore_destroy_and_return_if_it_is_marked_for_destruction @@ -180,7 +180,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase pirate = Pirate.new(:catchphrase => "Stop wastin' me time") pirate.ship_attributes = { :id => "" } - assert_nothing_raised(ActiveRecord::RecordNotFound) { pirate.save! } + assert_nothing_raised { pirate.save! } end def test_first_and_array_index_zero_methods_return_the_same_value_when_nested_attributes_are_set_to_update_existing_record @@ -511,7 +511,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase def test_should_not_destroy_an_existing_record_if_destroy_is_not_truthy [nil, '0', 0, 'false', false].each do |not_truth| @ship.update(pirate_attributes: { id: @ship.pirate.id, _destroy: not_truth }) - assert_nothing_raised(ActiveRecord::RecordNotFound) { @ship.pirate.reload } + assert_nothing_raised { @ship.pirate.reload } end end @@ -519,7 +519,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase Ship.accepts_nested_attributes_for :pirate, :allow_destroy => false, :reject_if => proc(&:empty?) @ship.update(pirate_attributes: { id: @ship.pirate.id, _destroy: '1' }) - assert_nothing_raised(ActiveRecord::RecordNotFound) { @ship.pirate.reload } + assert_nothing_raised { @ship.pirate.reload } ensure Ship.accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc(&:empty?) end @@ -536,7 +536,7 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase pirate = @ship.pirate @ship.attributes = { :pirate_attributes => { :id => pirate.id, '_destroy' => true } } - assert_nothing_raised(ActiveRecord::RecordNotFound) { Pirate.find(pirate.id) } + assert_nothing_raised { Pirate.find(pirate.id) } @ship.save assert_raise(ActiveRecord::RecordNotFound) { Pirate.find(pirate.id) } end @@ -713,7 +713,7 @@ module NestedAttributesOnACollectionAssociationTests end def test_should_not_assign_destroy_key_to_a_record - assert_nothing_raised ActiveRecord::UnknownAttributeError do + assert_nothing_raised do @pirate.send(association_setter, { 'foo' => { '_destroy' => '0' }}) end end @@ -748,8 +748,8 @@ module NestedAttributesOnACollectionAssociationTests end def test_should_raise_an_argument_error_if_something_else_than_a_hash_is_passed - assert_nothing_raised(ArgumentError) { @pirate.send(association_setter, {}) } - assert_nothing_raised(ArgumentError) { @pirate.send(association_setter, Hash.new) } + assert_nothing_raised { @pirate.send(association_setter, {}) } + assert_nothing_raised { @pirate.send(association_setter, Hash.new) } exception = assert_raise ArgumentError do @pirate.send(association_setter, "foo") @@ -824,7 +824,7 @@ module NestedAttributesOnACollectionAssociationTests def test_can_use_symbols_as_object_identifier @pirate.attributes = { :parrots_attributes => { :foo => { :name => 'Lovely Day' }, :bar => { :name => 'Blown Away' } } } - assert_nothing_raised(NoMethodError) { @pirate.save! } + assert_nothing_raised { @pirate.save! } end def test_numeric_column_changes_from_zero_to_no_empty_string diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index b918b36b94..e27a747730 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -130,7 +130,7 @@ class PrimaryKeysTest < ActiveRecord::TestCase end def test_supports_primary_key - assert_nothing_raised NoMethodError do + assert_nothing_raised do ActiveRecord::Base.connection.supports_primary_key? end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 090b885dd5..95e4230a58 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -358,6 +358,12 @@ class RelationTest < ActiveRecord::TestCase def test_finding_with_sanitized_order query = Tag.order(["field(id, ?)", [1,3,2]]).to_sql assert_match(/field\(id, 1,3,2\)/, query) + + query = Tag.order(["field(id, ?)", []]).to_sql + assert_match(/field\(id, NULL\)/, query) + + query = Tag.order(["field(id, ?)", nil]).to_sql + assert_match(/field\(id, NULL\)/, query) end def test_finding_with_order_limit_and_offset @@ -1273,6 +1279,16 @@ class RelationTest < ActiveRecord::TestCase assert posts.loaded? end + def test_to_a_should_dup_target + posts = Post.all + + original_size = posts.size + removed = posts.to_a.pop + + assert_equal original_size, posts.size + assert_includes posts.to_a, removed + end + def test_build posts = Post.all diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 970f6bcf4a..937b84bccc 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -98,8 +98,11 @@ class TimestampTest < ActiveRecord::TestCase task = Task.first previous_value = task.ending task.touch(:ending) + + now = Time.now.change(usec: 0) + assert_not_equal previous_value, task.ending - assert_in_delta Time.now, task.ending, 1 + assert_in_delta now, task.ending, 1 end def test_touching_an_attribute_updates_timestamp_with_given_time @@ -120,10 +123,12 @@ class TimestampTest < ActiveRecord::TestCase previous_ending = task.ending task.touch(:starting, :ending) + now = Time.now.change(usec: 0) + assert_not_equal previous_starting, task.starting assert_not_equal previous_ending, task.ending - assert_in_delta Time.now, task.starting, 1 - assert_in_delta Time.now, task.ending, 1 + assert_in_delta now, task.starting, 1 + assert_in_delta now, task.ending, 1 end def test_touching_a_record_without_timestamps_is_unexceptional diff --git a/activerecord/test/cases/validations/absence_validation_test.rb b/activerecord/test/cases/validations/absence_validation_test.rb index 180acbcb6a..c0b3750bcc 100644 --- a/activerecord/test/cases/validations/absence_validation_test.rb +++ b/activerecord/test/cases/validations/absence_validation_test.rb @@ -57,22 +57,6 @@ class AbsenceValidationTest < ActiveRecord::TestCase assert_nothing_raised { boy_klass.new(face: face_with_to_a).valid? } end - def test_does_not_validate_if_parent_record_is_validate_false - repair_validations(Interest) do - Interest.validates_absence_of(:topic) - interest = Interest.new(topic: Topic.new(title: "Math")) - interest.save!(validate: false) - assert interest.persisted? - - man = Man.new(interest_ids: [interest.id]) - man.save! - - assert_equal man.interests.size, 1 - assert interest.valid? - assert man.valid? - end - end - def test_validates_absence_of_virtual_attribute_on_model repair_validations(Interest) do Interest.send(:attr_accessor, :token) diff --git a/activerecord/test/cases/validations/length_validation_test.rb b/activerecord/test/cases/validations/length_validation_test.rb index 624aeaaddb..78263fd955 100644 --- a/activerecord/test/cases/validations/length_validation_test.rb +++ b/activerecord/test/cases/validations/length_validation_test.rb @@ -61,21 +61,7 @@ class LengthValidationTest < ActiveRecord::TestCase assert_equal pet_count, Pet.count end - def test_does_not_validate_length_of_if_parent_record_is_validate_false - @owner.validates_length_of :name, minimum: 1 - owner = @owner.new - owner.save!(validate: false) - assert owner.persisted? - - pet = Pet.new(owner_id: owner.id) - pet.save! - - assert_equal owner.pets.size, 1 - assert owner.valid? - assert pet.valid? - end - - def test_validates_presence_of_virtual_attribute_on_model + def test_validates_length_of_virtual_attribute_on_model repair_validations(Pet) do Pet.send(:attr_accessor, :nickname) Pet.validates_length_of(:name, minimum: 1) diff --git a/activerecord/test/cases/validations/presence_validation_test.rb b/activerecord/test/cases/validations/presence_validation_test.rb index 691f10a635..2ae30c7fd3 100644 --- a/activerecord/test/cases/validations/presence_validation_test.rb +++ b/activerecord/test/cases/validations/presence_validation_test.rb @@ -65,22 +65,6 @@ class PresenceValidationTest < ActiveRecord::TestCase assert_nothing_raised { s.valid? } end - def test_does_not_validate_presence_of_if_parent_record_is_validate_false - repair_validations(Interest) do - Interest.validates_presence_of(:topic) - interest = Interest.new - interest.save!(validate: false) - assert interest.persisted? - - man = Man.new(interest_ids: [interest.id]) - man.save! - - assert_equal man.interests.size, 1 - assert interest.valid? - assert man.valid? - end - end - def test_validates_presence_of_virtual_attribute_on_model repair_validations(Interest) do Interest.send(:attr_accessor, :abbreviation) @@ -95,4 +79,25 @@ class PresenceValidationTest < ActiveRecord::TestCase assert interest.invalid? end end + + def test_validations_run_on_persisted_record + repair_validations(Interest) do + interest = Interest.new + interest.save! + assert_predicate interest, :valid? + + Interest.validates_presence_of(:topic) + + assert_not_predicate interest, :valid? + end + end + + def test_validates_prescence_with_on_context + repair_validations(Interest) do + Interest.validates_presence_of(:topic, on: :required_name) + interest = Interest.new + interest.save! + assert_not interest.valid?(:required_name) + end + end end diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index e601c53dbf..8abb6c9844 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -5,6 +5,7 @@ require 'models/warehouse_thing' require 'models/guid' require 'models/event' require 'models/dashboard' +require 'models/uuid_item' class Wizard < ActiveRecord::Base self.abstract_class = true @@ -48,6 +49,10 @@ class BigIntReverseTest < ActiveRecord::Base validates :engines_count, uniqueness: true end +class CoolTopic < Topic + validates_uniqueness_of :id +end + class UniquenessValidationTest < ActiveRecord::TestCase INT_MAX_VALUE = 2147483647 @@ -412,23 +417,6 @@ class UniquenessValidationTest < ActiveRecord::TestCase assert topic.valid? end - def test_does_not_validate_uniqueness_of_if_parent_record_is_validate_false - Reply.validates_uniqueness_of(:content) - - Reply.create!(content: "Topic Title") - - reply = Reply.new(content: "Topic Title") - reply.save!(validate: false) - assert reply.persisted? - - topic = Topic.new(reply_ids: [reply.id]) - topic.save! - - assert_equal topic.replies.size, 1 - assert reply.valid? - assert topic.valid? - end - def test_validate_uniqueness_of_custom_primary_key klass = Class.new(ActiveRecord::Base) do self.table_name = "keyboards" @@ -480,4 +468,25 @@ class UniquenessValidationTest < ActiveRecord::TestCase assert t.valid?, "Should be valid" assert t.save, "Should still save t as unique" end + + def test_validate_uniqueness_uuid + skip unless current_adapter?(:PostgreSQLAdapter) + item = UuidItem.create!(uuid: SecureRandom.uuid, title: 'item1') + item.update(title: 'item1-title2') + assert_empty item.errors + + item2 = UuidValidatingItem.create!(uuid: SecureRandom.uuid, title: 'item2') + item2.update(title: 'item2-title2') + assert_empty item2.errors + end + + def test_validate_uniqueness_regular_id + item = CoolTopic.create!(title: 'MyItem') + assert_empty item.errors + + item2 = CoolTopic.new(id: item.id, title: 'MyItem2') + refute item2.valid? + + assert_equal(["has already been taken"], item2.errors[:id]) + end end diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index d04f4f7ce7..85e33d2218 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -130,7 +130,7 @@ class ValidationsTest < ActiveRecord::TestCase def test_validates_acceptance_of_with_non_existent_table Object.const_set :IncorporealModel, Class.new(ActiveRecord::Base) - assert_nothing_raised ActiveRecord::StatementInvalid do + assert_nothing_raised do IncorporealModel.validates_acceptance_of(:incorporeal_column) end end diff --git a/activerecord/test/models/uuid_item.rb b/activerecord/test/models/uuid_item.rb new file mode 100644 index 0000000000..2353e40213 --- /dev/null +++ b/activerecord/test/models/uuid_item.rb @@ -0,0 +1,6 @@ +class UuidItem < ActiveRecord::Base +end + +class UuidValidatingItem < UuidItem + validates_uniqueness_of :uuid +end diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb index 3a5d73a0ed..24713f722a 100644 --- a/activerecord/test/schema/postgresql_specific_schema.rb +++ b/activerecord/test/schema/postgresql_specific_schema.rb @@ -106,4 +106,9 @@ _SQL t.integer :big_int_data_points, limit: 8, array: true t.decimal :decimal_array_default, array: true, default: [1.23, 3.45] end + + create_table :uuid_items, force: true, id: false do |t| + t.uuid :uuid, primary_key: true + t.string :title + end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index db8d279cff..2a5d203813 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,12 @@ +* Deprecate arguments on `assert_nothing_raised`. + + `assert_nothing_raised` does not assert the arguments that have been passed + in (usually a specific exception class) since the method only yields the + block. So as not to confuse the users that the arguments have meaning, they + are being deprecated. + + *Tara Scherner de la Fuente* + * Make `benchmark('something', silence: true)` actually work *DHH* @@ -6,7 +15,7 @@ `#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. diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 610105f41c..1c63e8a93f 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -333,21 +333,19 @@ module ActiveSupport options = names.extract_options! options = merged_options(options) - instrument_multi(:read, names, options) do |payload| - results = {} - names.each do |name| - key = normalize_key(name, options) - entry = read_entry(key, options) - if entry - if entry.expired? - delete_entry(key, options) - else - results[name] = entry.value - end + results = {} + names.each do |name| + key = normalize_key(name, options) + entry = read_entry(key, options) + if entry + if entry.expired? + delete_entry(key, options) + else + results[name] = entry.value end end - results end + results end # Fetches data from the cache, using the given keys. If there is data in @@ -555,17 +553,6 @@ module ActiveSupport ActiveSupport::Notifications.instrument("cache_#{operation}.active_support", payload){ yield(payload) } end - def instrument_multi(operation, keys, options = nil) - log do - formatted_keys = keys.map { |k| "- #{k}" }.join("\n") - "Caches multi #{operation}:\n#{formatted_keys}#{options.blank? ? "" : " (#{options.inspect})"}" - end - - payload = { key: keys } - payload.merge!(options) if options.is_a?(Hash) - ActiveSupport::Notifications.instrument("cache_#{operation}_multi.active_support", payload) { yield(payload) } - end - def log return unless logger && logger.debug? && !silence? logger.debug(yield) diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index 174913365a..2ca4b51efa 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -96,16 +96,14 @@ module ActiveSupport options = names.extract_options! options = merged_options(options) - instrument_multi(:read, names, options) do - keys_to_names = Hash[names.map{|name| [normalize_key(name, options), name]}] - raw_values = @data.get_multi(keys_to_names.keys, :raw => true) - values = {} - raw_values.each do |key, value| - entry = deserialize_entry(value) - values[keys_to_names[key]] = entry.value unless entry.expired? - end - values + keys_to_names = Hash[names.map{|name| [normalize_key(name, options), name]}] + raw_values = @data.get_multi(keys_to_names.keys, :raw => true) + values = {} + raw_values.each do |key, value| + entry = deserialize_entry(value) + values[keys_to_names[key]] = entry.value unless entry.expired? end + values end # Increment a cached value. This method uses the memcached incr atomic diff --git a/activesupport/lib/active_support/core_ext/numeric/conversions.rb b/activesupport/lib/active_support/core_ext/numeric/conversions.rb index 9d832897ed..b25925b9d4 100644 --- a/activesupport/lib/active_support/core_ext/numeric/conversions.rb +++ b/activesupport/lib/active_support/core_ext/numeric/conversions.rb @@ -15,72 +15,72 @@ module ActiveSupport::NumericWithFormat # ==== Examples # # Phone Numbers: - # 5551234.to_s(:phone) # => 555-1234 - # 1235551234.to_s(:phone) # => 123-555-1234 - # 1235551234.to_s(:phone, area_code: true) # => (123) 555-1234 - # 1235551234.to_s(:phone, delimiter: ' ') # => 123 555 1234 - # 1235551234.to_s(:phone, area_code: true, extension: 555) # => (123) 555-1234 x 555 - # 1235551234.to_s(:phone, country_code: 1) # => +1-123-555-1234 + # 5551234.to_s(:phone) # => "555-1234" + # 1235551234.to_s(:phone) # => "123-555-1234" + # 1235551234.to_s(:phone, area_code: true) # => "(123) 555-1234" + # 1235551234.to_s(:phone, delimiter: ' ') # => "123 555 1234" + # 1235551234.to_s(:phone, area_code: true, extension: 555) # => "(123) 555-1234 x 555" + # 1235551234.to_s(:phone, country_code: 1) # => "+1-123-555-1234" # 1235551234.to_s(:phone, country_code: 1, extension: 1343, delimiter: '.') - # # => +1.123.555.1234 x 1343 + # # => "+1.123.555.1234 x 1343" # # Currency: - # 1234567890.50.to_s(:currency) # => $1,234,567,890.50 - # 1234567890.506.to_s(:currency) # => $1,234,567,890.51 - # 1234567890.506.to_s(:currency, precision: 3) # => $1,234,567,890.506 - # 1234567890.506.to_s(:currency, locale: :fr) # => 1 234 567 890,51 € + # 1234567890.50.to_s(:currency) # => "$1,234,567,890.50" + # 1234567890.506.to_s(:currency) # => "$1,234,567,890.51" + # 1234567890.506.to_s(:currency, precision: 3) # => "$1,234,567,890.506" + # 1234567890.506.to_s(:currency, locale: :fr) # => "1 234 567 890,51 €" # -1234567890.50.to_s(:currency, negative_format: '(%u%n)') - # # => ($1,234,567,890.50) + # # => "($1,234,567,890.50)" # 1234567890.50.to_s(:currency, unit: '£', separator: ',', delimiter: '') - # # => £1234567890,50 + # # => "£1234567890,50" # 1234567890.50.to_s(:currency, unit: '£', separator: ',', delimiter: '', format: '%n %u') - # # => 1234567890,50 £ + # # => "1234567890,50 £" # # Percentage: - # 100.to_s(:percentage) # => 100.000% - # 100.to_s(:percentage, precision: 0) # => 100% - # 1000.to_s(:percentage, delimiter: '.', separator: ',') # => 1.000,000% - # 302.24398923423.to_s(:percentage, precision: 5) # => 302.24399% - # 1000.to_s(:percentage, locale: :fr) # => 1 000,000% - # 100.to_s(:percentage, format: '%n %') # => 100.000 % + # 100.to_s(:percentage) # => "100.000%" + # 100.to_s(:percentage, precision: 0) # => "100%" + # 1000.to_s(:percentage, delimiter: '.', separator: ',') # => "1.000,000%" + # 302.24398923423.to_s(:percentage, precision: 5) # => "302.24399%" + # 1000.to_s(:percentage, locale: :fr) # => "1 000,000%" + # 100.to_s(:percentage, format: '%n %') # => "100.000 %" # # Delimited: - # 12345678.to_s(:delimited) # => 12,345,678 - # 12345678.05.to_s(:delimited) # => 12,345,678.05 - # 12345678.to_s(:delimited, delimiter: '.') # => 12.345.678 - # 12345678.to_s(:delimited, delimiter: ',') # => 12,345,678 - # 12345678.05.to_s(:delimited, separator: ' ') # => 12,345,678 05 - # 12345678.05.to_s(:delimited, locale: :fr) # => 12 345 678,05 + # 12345678.to_s(:delimited) # => "12,345,678" + # 12345678.05.to_s(:delimited) # => "12,345,678.05" + # 12345678.to_s(:delimited, delimiter: '.') # => "12.345.678" + # 12345678.to_s(:delimited, delimiter: ',') # => "12,345,678" + # 12345678.05.to_s(:delimited, separator: ' ') # => "12,345,678 05" + # 12345678.05.to_s(:delimited, locale: :fr) # => "12 345 678,05" # 98765432.98.to_s(:delimited, delimiter: ' ', separator: ',') - # # => 98 765 432,98 + # # => "98 765 432,98" # # Rounded: - # 111.2345.to_s(:rounded) # => 111.235 - # 111.2345.to_s(:rounded, precision: 2) # => 111.23 - # 13.to_s(:rounded, precision: 5) # => 13.00000 - # 389.32314.to_s(:rounded, precision: 0) # => 389 - # 111.2345.to_s(:rounded, significant: true) # => 111 - # 111.2345.to_s(:rounded, precision: 1, significant: true) # => 100 - # 13.to_s(:rounded, precision: 5, significant: true) # => 13.000 - # 111.234.to_s(:rounded, locale: :fr) # => 111,234 + # 111.2345.to_s(:rounded) # => "111.235" + # 111.2345.to_s(:rounded, precision: 2) # => "111.23" + # 13.to_s(:rounded, precision: 5) # => "13.00000" + # 389.32314.to_s(:rounded, precision: 0) # => "389" + # 111.2345.to_s(:rounded, significant: true) # => "111" + # 111.2345.to_s(:rounded, precision: 1, significant: true) # => "100" + # 13.to_s(:rounded, precision: 5, significant: true) # => "13.000" + # 111.234.to_s(:rounded, locale: :fr) # => "111,234" # 13.to_s(:rounded, precision: 5, significant: true, strip_insignificant_zeros: true) - # # => 13 - # 389.32314.to_s(:rounded, precision: 4, significant: true) # => 389.3 + # # => "13" + # 389.32314.to_s(:rounded, precision: 4, significant: true) # => "389.3" # 1111.2345.to_s(:rounded, precision: 2, separator: ',', delimiter: '.') - # # => 1.111,23 + # # => "1.111,23" # # Human-friendly size in Bytes: - # 123.to_s(:human_size) # => 123 Bytes - # 1234.to_s(:human_size) # => 1.21 KB - # 12345.to_s(:human_size) # => 12.1 KB - # 1234567.to_s(:human_size) # => 1.18 MB - # 1234567890.to_s(:human_size) # => 1.15 GB - # 1234567890123.to_s(:human_size) # => 1.12 TB - # 1234567890123456.to_s(:human_size) # => 1.1 PB - # 1234567890123456789.to_s(:human_size) # => 1.07 EB - # 1234567.to_s(:human_size, precision: 2) # => 1.2 MB - # 483989.to_s(:human_size, precision: 2) # => 470 KB - # 1234567.to_s(:human_size, precision: 2, separator: ',') # => 1,2 MB + # 123.to_s(:human_size) # => "123 Bytes" + # 1234.to_s(:human_size) # => "1.21 KB" + # 12345.to_s(:human_size) # => "12.1 KB" + # 1234567.to_s(:human_size) # => "1.18 MB" + # 1234567890.to_s(:human_size) # => "1.15 GB" + # 1234567890123.to_s(:human_size) # => "1.12 TB" + # 1234567890123456.to_s(:human_size) # => "1.1 PB" + # 1234567890123456789.to_s(:human_size) # => "1.07 EB" + # 1234567.to_s(:human_size, precision: 2) # => "1.2 MB" + # 483989.to_s(:human_size, precision: 2) # => "470 KB" + # 1234567.to_s(:human_size, precision: 2, separator: ',') # => "1,2 MB" # 1234567890123.to_s(:human_size, precision: 5) # => "1.1228 TB" # 524288000.to_s(:human_size, precision: 5) # => "500 MB" # diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index 64d9e71f37..55628f0313 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -29,17 +29,17 @@ module ActiveSupport # number. # ==== Examples # - # number_to_phone(5551234) # => 555-1234 - # number_to_phone('5551234') # => 555-1234 - # number_to_phone(1235551234) # => 123-555-1234 - # number_to_phone(1235551234, area_code: true) # => (123) 555-1234 - # number_to_phone(1235551234, delimiter: ' ') # => 123 555 1234 - # number_to_phone(1235551234, area_code: true, extension: 555) # => (123) 555-1234 x 555 - # number_to_phone(1235551234, country_code: 1) # => +1-123-555-1234 - # number_to_phone('123a456') # => 123a456 + # number_to_phone(5551234) # => "555-1234" + # number_to_phone('5551234') # => "555-1234" + # number_to_phone(1235551234) # => "123-555-1234" + # number_to_phone(1235551234, area_code: true) # => "(123) 555-1234" + # number_to_phone(1235551234, delimiter: ' ') # => "123 555 1234" + # number_to_phone(1235551234, area_code: true, extension: 555) # => "(123) 555-1234 x 555" + # number_to_phone(1235551234, country_code: 1) # => "+1-123-555-1234" + # number_to_phone('123a456') # => "123a456" # # number_to_phone(1235551234, country_code: 1, extension: 1343, delimiter: '.') - # # => +1.123.555.1234 x 1343 + # # => "+1.123.555.1234 x 1343" def number_to_phone(number, options = {}) NumberToPhoneConverter.convert(number, options) end @@ -78,18 +78,18 @@ module ActiveSupport # # ==== Examples # - # number_to_currency(1234567890.50) # => $1,234,567,890.50 - # number_to_currency(1234567890.506) # => $1,234,567,890.51 - # number_to_currency(1234567890.506, precision: 3) # => $1,234,567,890.506 - # number_to_currency(1234567890.506, locale: :fr) # => 1 234 567 890,51 € - # number_to_currency('123a456') # => $123a456 + # number_to_currency(1234567890.50) # => "$1,234,567,890.50" + # number_to_currency(1234567890.506) # => "$1,234,567,890.51" + # number_to_currency(1234567890.506, precision: 3) # => "$1,234,567,890.506" + # number_to_currency(1234567890.506, locale: :fr) # => "1 234 567 890,51 €" + # number_to_currency('123a456') # => "$123a456" # # number_to_currency(-1234567890.50, negative_format: '(%u%n)') - # # => ($1,234,567,890.50) + # # => "($1,234,567,890.50)" # number_to_currency(1234567890.50, unit: '£', separator: ',', delimiter: '') - # # => £1234567890,50 + # # => "£1234567890,50" # number_to_currency(1234567890.50, unit: '£', separator: ',', delimiter: '', format: '%n %u') - # # => 1234567890,50 £ + # # => "1234567890,50 £" def number_to_currency(number, options = {}) NumberToCurrencyConverter.convert(number, options) end @@ -118,15 +118,15 @@ module ActiveSupport # # ==== Examples # - # number_to_percentage(100) # => 100.000% - # number_to_percentage('98') # => 98.000% - # number_to_percentage(100, precision: 0) # => 100% - # number_to_percentage(1000, delimiter: '.', separator: ',') # => 1.000,000% - # number_to_percentage(302.24398923423, precision: 5) # => 302.24399% - # number_to_percentage(1000, locale: :fr) # => 1000,000% - # number_to_percentage(1000, precision: nil) # => 1000% - # number_to_percentage('98a') # => 98a% - # number_to_percentage(100, format: '%n %') # => 100.000 % + # number_to_percentage(100) # => "100.000%" + # number_to_percentage('98') # => "98.000%" + # number_to_percentage(100, precision: 0) # => "100%" + # number_to_percentage(1000, delimiter: '.', separator: ',') # => "1.000,000%" + # number_to_percentage(302.24398923423, precision: 5) # => "302.24399%" + # number_to_percentage(1000, locale: :fr) # => "1000,000%" + # number_to_percentage(1000, precision: nil) # => "1000%" + # number_to_percentage('98a') # => "98a%" + # number_to_percentage(100, format: '%n %') # => "100.000 %" def number_to_percentage(number, options = {}) NumberToPercentageConverter.convert(number, options) end @@ -149,19 +149,19 @@ module ActiveSupport # # ==== Examples # - # number_to_delimited(12345678) # => 12,345,678 - # number_to_delimited('123456') # => 123,456 - # number_to_delimited(12345678.05) # => 12,345,678.05 - # number_to_delimited(12345678, delimiter: '.') # => 12.345.678 - # number_to_delimited(12345678, delimiter: ',') # => 12,345,678 - # number_to_delimited(12345678.05, separator: ' ') # => 12,345,678 05 - # number_to_delimited(12345678.05, locale: :fr) # => 12 345 678,05 - # number_to_delimited('112a') # => 112a + # number_to_delimited(12345678) # => "12,345,678" + # number_to_delimited('123456') # => "123,456" + # number_to_delimited(12345678.05) # => "12,345,678.05" + # number_to_delimited(12345678, delimiter: '.') # => "12.345.678" + # number_to_delimited(12345678, delimiter: ',') # => "12,345,678" + # number_to_delimited(12345678.05, separator: ' ') # => "12,345,678 05" + # number_to_delimited(12345678.05, locale: :fr) # => "12 345 678,05" + # number_to_delimited('112a') # => "112a" # number_to_delimited(98765432.98, delimiter: ' ', separator: ',') - # # => 98 765 432,98 + # # => "98 765 432,98" # number_to_delimited("123456.78", # delimiter_pattern: /(\d+?)(?=(\d\d)+(\d)(?!\d))/) - # # => 1,23,456.78 + # # => "1,23,456.78" def number_to_delimited(number, options = {}) NumberToDelimitedConverter.convert(number, options) end @@ -190,22 +190,22 @@ module ActiveSupport # # ==== Examples # - # number_to_rounded(111.2345) # => 111.235 - # number_to_rounded(111.2345, precision: 2) # => 111.23 - # number_to_rounded(13, precision: 5) # => 13.00000 - # number_to_rounded(389.32314, precision: 0) # => 389 - # number_to_rounded(111.2345, significant: true) # => 111 - # number_to_rounded(111.2345, precision: 1, significant: true) # => 100 - # number_to_rounded(13, precision: 5, significant: true) # => 13.000 - # number_to_rounded(13, precision: nil) # => 13 - # number_to_rounded(111.234, locale: :fr) # => 111,234 + # number_to_rounded(111.2345) # => "111.235" + # number_to_rounded(111.2345, precision: 2) # => "111.23" + # number_to_rounded(13, precision: 5) # => "13.00000" + # number_to_rounded(389.32314, precision: 0) # => "389" + # number_to_rounded(111.2345, significant: true) # => "111" + # number_to_rounded(111.2345, precision: 1, significant: true) # => "100" + # number_to_rounded(13, precision: 5, significant: true) # => "13.000" + # number_to_rounded(13, precision: nil) # => "13" + # number_to_rounded(111.234, locale: :fr) # => "111,234" # # number_to_rounded(13, precision: 5, significant: true, strip_insignificant_zeros: true) - # # => 13 + # # => "13" # - # number_to_rounded(389.32314, precision: 4, significant: true) # => 389.3 + # number_to_rounded(389.32314, precision: 4, significant: true) # => "389.3" # number_to_rounded(1111.2345, precision: 2, separator: ',', delimiter: '.') - # # => 1.111,23 + # # => "1.111,23" def number_to_rounded(number, options = {}) NumberToRoundedConverter.convert(number, options) end @@ -237,17 +237,17 @@ module ActiveSupport # # ==== Examples # - # number_to_human_size(123) # => 123 Bytes - # number_to_human_size(1234) # => 1.21 KB - # number_to_human_size(12345) # => 12.1 KB - # number_to_human_size(1234567) # => 1.18 MB - # number_to_human_size(1234567890) # => 1.15 GB - # number_to_human_size(1234567890123) # => 1.12 TB - # number_to_human_size(1234567890123456) # => 1.1 PB - # number_to_human_size(1234567890123456789) # => 1.07 EB - # number_to_human_size(1234567, precision: 2) # => 1.2 MB - # number_to_human_size(483989, precision: 2) # => 470 KB - # number_to_human_size(1234567, precision: 2, separator: ',') # => 1,2 MB + # number_to_human_size(123) # => "123 Bytes" + # number_to_human_size(1234) # => "1.21 KB" + # number_to_human_size(12345) # => "12.1 KB" + # number_to_human_size(1234567) # => "1.18 MB" + # number_to_human_size(1234567890) # => "1.15 GB" + # number_to_human_size(1234567890123) # => "1.12 TB" + # number_to_human_size(1234567890123456) # => "1.1 PB" + # number_to_human_size(1234567890123456789) # => "1.07 EB" + # number_to_human_size(1234567, precision: 2) # => "1.2 MB" + # number_to_human_size(483989, precision: 2) # => "470 KB" + # number_to_human_size(1234567, precision: 2, separator: ',') # => "1,2 MB" # number_to_human_size(1234567890123, precision: 5) # => "1.1228 TB" # number_to_human_size(524288000, precision: 5) # => "500 MB" def number_to_human_size(number, options = {}) diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index 4dc84e4a59..1fc12d0bc1 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -75,6 +75,11 @@ module ActiveSupport # perform_service(param: 'no_exception') # end def assert_nothing_raised(*args) + if args.present? + ActiveSupport::Deprecation.warn( + "Passing arguments to assert_nothing_raised " \ + "is deprecated and will be removed in Rails 5.1.") + end yield end end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 4a299429f3..9e744afb2b 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -836,7 +836,7 @@ class FileStoreTest < ActiveSupport::TestCase # If nothing has been stored in the cache, there is a chance the cache directory does not yet exist # Ensure delete_matched gracefully handles this case def test_delete_matched_when_cache_directory_does_not_exist - assert_nothing_raised(Exception) do + assert_nothing_raised do ActiveSupport::Cache::FileStore.new('/test/cache/directory').delete_matched(/does_not_exist/) end end @@ -844,7 +844,7 @@ class FileStoreTest < ActiveSupport::TestCase def test_delete_does_not_delete_empty_parent_dir sub_cache_dir = File.join(cache_dir, 'subdir/') sub_cache_store = ActiveSupport::Cache::FileStore.new(sub_cache_dir) - assert_nothing_raised(Exception) do + assert_nothing_raised do assert sub_cache_store.write('foo', 'bar') assert sub_cache_store.delete('foo') end @@ -1151,15 +1151,6 @@ class CacheStoreLoggerTest < ActiveSupport::TestCase @cache.mute { @cache.fetch('foo') { 'bar' } } assert @buffer.string.blank? end - - def test_multi_read_loggin - @cache.write 'hello', 'goodbye' - @cache.write 'world', 'earth' - - @cache.read_multi('hello', 'world') - - assert_match "Caches multi read:\n- hello\n- world", @buffer.string - end end class CacheEntryTest < ActiveSupport::TestCase diff --git a/activesupport/test/core_ext/date_time_ext_test.rb b/activesupport/test/core_ext/date_time_ext_test.rb index 6fe38c45ec..b183a20e0d 100644 --- a/activesupport/test/core_ext/date_time_ext_test.rb +++ b/activesupport/test/core_ext/date_time_ext_test.rb @@ -186,6 +186,10 @@ class DateTimeExtCalculationsTest < ActiveSupport::TestCase assert_equal DateTime.civil(2006,11,15), DateTime.civil(2006,11,23,0,0,0).last_week(:wednesday) end + def test_date_time_should_have_correct_last_week_for_leap_year + assert_equal DateTime.civil(2016, 2, 29), DateTime.civil(2016, 3, 7).last_week + end + def test_last_month_on_31st assert_equal DateTime.civil(2004, 2, 29), DateTime.civil(2004, 3, 31).last_month end diff --git a/activesupport/test/core_ext/marshal_test.rb b/activesupport/test/core_ext/marshal_test.rb index 825df439a5..5427837d19 100644 --- a/activesupport/test/core_ext/marshal_test.rb +++ b/activesupport/test/core_ext/marshal_test.rb @@ -96,7 +96,7 @@ class MarshalTest < ActiveSupport::TestCase Marshal.load(dumped) end - assert_nothing_raised("EM failed to load while we expect only SomeClass to fail loading") do + assert_nothing_raised do EM.new end diff --git a/activesupport/test/core_ext/time_ext_test.rb b/activesupport/test/core_ext/time_ext_test.rb index e45df63fd5..d8bb38621b 100644 --- a/activesupport/test/core_ext/time_ext_test.rb +++ b/activesupport/test/core_ext/time_ext_test.rb @@ -392,7 +392,7 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase assert_equal Time.local(2005,1,2,11,22,33, 8), Time.local(2005,1,2,11,22,33,44).change(:usec => 8) assert_equal Time.local(2005,1,2,11,22,33, 8), Time.local(2005,1,2,11,22,33,2).change(:nsec => 8000) assert_raise(ArgumentError) { Time.local(2005,1,2,11,22,33, 8).change(:usec => 1, :nsec => 1) } - assert_nothing_raised(ArgumentError) { Time.new(2015, 5, 9, 10, 00, 00, '+03:00').change(nsec: 999999999) } + assert_nothing_raised { Time.new(2015, 5, 9, 10, 00, 00, '+03:00').change(nsec: 999999999) } end def test_utc_change diff --git a/guides/source/5_0_release_notes.md b/guides/source/5_0_release_notes.md index d380ba8fce..f5abbd0cd4 100644 --- a/guides/source/5_0_release_notes.md +++ b/guides/source/5_0_release_notes.md @@ -294,6 +294,9 @@ Please refer to the [Changelog][action-view] for detailed changes. button on submit to prevent double submits. ([Pull Request](https://github.com/rails/rails/pull/21135)) +* Downcase model name in form submit tags rather than humanize. + ([Pull Request](https://github.com/rails/rails/pull/22764)) + Action Mailer ------------- diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index 5dd54bf8ad..b6c612794c 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -335,8 +335,8 @@ include the 'data-turbolinks-track' option which causes turbolinks to check if an asset has been updated and if so loads it into the page: ```erb -<%= stylesheet_link_tag "application", media: "all", "data-turbolinks-track" => true %> -<%= javascript_include_tag "application", "data-turbolinks-track" => true %> +<%= stylesheet_link_tag "application", media: "all", "data-turbolinks-track" => "reload" %> +<%= javascript_include_tag "application", "data-turbolinks-track" => "reload" %> ``` In regular views you can access images in the `public/assets/images` directory diff --git a/guides/source/form_helpers.md b/guides/source/form_helpers.md index 2a289dd33a..422bc647ef 100644 --- a/guides/source/form_helpers.md +++ b/guides/source/form_helpers.md @@ -317,7 +317,7 @@ The Article model is directly available to users of the application, so - follow resources :articles ``` -TIP: Declaring a resource has a number of side-affects. See [Rails Routing From the Outside In](routing.html#resource-routing-the-rails-default) for more information on setting up and using resources. +TIP: Declaring a resource has a number of side effects. See [Rails Routing From the Outside In](routing.html#resource-routing-the-rails-default) for more information on setting up and using resources. When dealing with RESTful resources, calls to `form_for` can get significantly easier if you rely on **record identification**. In short, you can just pass the model instance and have Rails figure out model name and the rest: diff --git a/guides/source/testing.md b/guides/source/testing.md index 7a9a30f7ac..09eec7a64c 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -287,7 +287,7 @@ specify to make your test failure messages clearer. | `assert_not_in_delta( expected, actual, [delta], [msg] )` | Ensures that the numbers `expected` and `actual` are not within `delta` of each other.| | `assert_throws( symbol, [msg] ) { block }` | Ensures that the given block throws the symbol.| | `assert_raises( exception1, exception2, ... ) { block }` | Ensures that the given block raises one of the given exceptions.| -| `assert_nothing_raised( exception1, exception2, ... ) { block }` | Ensures that the given block doesn't raise one of the given exceptions.| +| `assert_nothing_raised { block }` | Ensures that the given block doesn't raise any exceptions.| | `assert_instance_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class`.| | `assert_not_instance_of( class, obj, [msg] )` | Ensures that `obj` is not an instance of `class`.| | `assert_kind_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class` or is descending from it.| diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index 26ff5da7a3..c58aee96db 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -350,8 +350,8 @@ $("<%= escape_javascript(render @user) %>").appendTo("#users"); Turbolinks ---------- -Rails 4 ships with the [Turbolinks gem](https://github.com/turbolinks/turbolinks). -This gem uses Ajax to speed up page rendering in most applications. +Rails ships with the [Turbolinks library](https://github.com/turbolinks/turbolinks), +which uses Ajax to speed up page rendering in most applications. ### How Turbolinks Works @@ -364,14 +364,14 @@ will then use PushState to change the URL to the correct one, preserving refresh semantics and giving you pretty URLs. The only thing you have to do to enable Turbolinks is have it in your Gemfile, -and put `//= require turbolinks` in your CoffeeScript manifest, which is usually +and put `//= require turbolinks` in your JavaScript manifest, which is usually `app/assets/javascripts/application.js`. -If you want to disable Turbolinks for certain links, add a `data-no-turbolink` +If you want to disable Turbolinks for certain links, add a `data-turbolinks="false"` attribute to the tag: ```html -<a href="..." data-no-turbolink>No turbolinks here</a>. +<a href="..." data-turbolinks="false">No turbolinks here</a>. ``` ### Page Change Events @@ -389,7 +389,7 @@ event that this relies on will not be fired. If you have code that looks like this, you must change your code to do this instead: ```coffeescript -$(document).on "page:change", -> +$(document).on "turbolinks:load", -> alert "page has loaded!" ``` diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb index 411cdbad19..64ec539564 100644 --- a/railties/lib/rails/application/finisher.rb +++ b/railties/lib/rails/application/finisher.rb @@ -22,10 +22,10 @@ module Rails initializer :add_builtin_route do |app| if Rails.env.development? app.routes.append do - get '/rails/info/properties' => "rails/info#properties" - get '/rails/info/routes' => "rails/info#routes" - get '/rails/info' => "rails/info#index" - get '/' => "rails/welcome#index" + get '/rails/info/properties' => "rails/info#properties", internal: true + get '/rails/info/routes' => "rails/info#routes", internal: true + get '/rails/info' => "rails/info#index", internal: true + get '/' => "rails/welcome#index", internal: true end end end diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 9adfcc6ee7..dd63cdbf81 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -328,8 +328,8 @@ module Rails "Use #{options[:javascript]} as the JavaScript library") unless options[:skip_turbolinks] - gems << GemfileEntry.version("turbolinks", nil, - "Turbolinks makes following links in your web application faster. Read more: https://github.com/turbolinks/turbolinks") + gems << GemfileEntry.version("turbolinks", "~> 5.x", + "Turbolinks makes navigating your web application faster. Read more: https://github.com/turbolinks/turbolinks") end gems diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 885f0c20f6..07d38605a2 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -91,6 +91,7 @@ module Rails cookie_serializer_config_exist = File.exist?('config/initializers/cookies_serializer.rb') callback_terminator_config_exist = File.exist?('config/initializers/callback_terminator.rb') active_record_belongs_to_required_by_default_config_exist = File.exist?('config/initializers/active_record_belongs_to_required_by_default.rb') + action_cable_config_exist = File.exist?('config/cable.yml') config @@ -105,6 +106,10 @@ module Rails unless active_record_belongs_to_required_by_default_config_exist remove_file 'config/initializers/active_record_belongs_to_required_by_default.rb' end + + unless action_cable_config_exist + template 'config/cable.yml' + end end def database_yml diff --git a/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt b/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt index 68b5c051b2..ec781721cb 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt +++ b/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt @@ -11,8 +11,8 @@ <%%= stylesheet_link_tag 'application', media: 'all' %> <%- else -%> <%- if gemfile_entries.any? { |m| m.name == 'turbolinks' } -%> - <%%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track' => true %> - <%%= javascript_include_tag 'application', 'data-turbolinks-track' => true %> + <%%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track' => 'reload' %> + <%%= javascript_include_tag 'application', 'data-turbolinks-track' => 'reload' %> <%- else -%> <%%= stylesheet_link_tag 'application', media: 'all' %> <%%= javascript_include_tag 'application' %> 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 d4e2b1c756..8e9890378a 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 @@ -15,12 +15,22 @@ Rails.application.configure do # Enable/disable caching. By default caching is disabled. if Rails.root.join('tmp/caching-dev.txt').exist? config.action_controller.perform_caching = true + + <%- unless options.skip_action_mailer? -%> + config.action_mailer.perform_caching = false + <%- end -%> + config.cache_store = :memory_store config.public_file_server.headers = { 'Cache-Control' => 'public, max-age=172800' } else config.action_controller.perform_caching = false + + <%- unless options.skip_action_mailer? -%> + config.action_mailer.perform_caching = false + <%- end -%> + config.cache_store = :null_store end <%- unless options.skip_action_mailer? -%> diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt index 82509f5ef5..236e42fcd7 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt @@ -60,6 +60,10 @@ Rails.application.configure do # require 'syslog/logger' # config.logger = ActiveSupport::TaggedLogging.new(Syslog::Logger.new 'app-name') + if ENV["RAILS_LOG_TO_STDOUT"].present? + config.logger = ActiveSupport::TaggedLogging.new(Logger.new(STDOUT)) + end + # Use a different cache store in production. # config.cache_store = :mem_cache_store @@ -67,6 +71,7 @@ Rails.application.configure do # config.active_job.queue_adapter = :resque # config.active_job.queue_name_prefix = "<%= app_name %>_#{Rails.env}" <%- unless options.skip_action_mailer? -%> + config.action_mailer.perform_caching = false # Ignore bad email addresses and do not raise email delivery errors. # Set this to true and configure the email server for immediate delivery to raise delivery errors. 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 e8c8b00669..42fee3b036 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt @@ -28,6 +28,7 @@ Rails.application.configure do # Disable request forgery protection in test environment. config.action_controller.allow_forgery_protection = false <%- unless options.skip_action_mailer? -%> + config.action_mailer.perform_caching = false # Tell Action Mailer not to deliver emails to the real world. # The :test delivery method accumulates sent emails in the diff --git a/railties/lib/rails/test_unit/minitest_plugin.rb b/railties/lib/rails/test_unit/minitest_plugin.rb index 03c705ffef..f22139490b 100644 --- a/railties/lib/rails/test_unit/minitest_plugin.rb +++ b/railties/lib/rails/test_unit/minitest_plugin.rb @@ -1,6 +1,7 @@ require "active_support/core_ext/module/attribute_accessors" require "rails/test_unit/reporter" require "rails/test_unit/test_requirer" +require 'shellwords' module Minitest class SuppressedSummaryReporter < SummaryReporter @@ -60,11 +61,13 @@ module Minitest # as the patterns would also contain the other Rake tasks. def self.rake_run(patterns) # :nodoc: @rake_patterns = patterns - passed = run + passed = run(Shellwords.split(ENV['TESTOPTS'] || '')) exit passed unless passed passed end + # Owes great inspiration to test runner trailblazers like RSpec, + # minitest-reporters, maxitest and others. def self.plugin_rails_init(options) self.run_with_rails_extension = true diff --git a/railties/lib/rails/test_unit/reporter.rb b/railties/lib/rails/test_unit/reporter.rb index e81bd9df7e..4086d5b731 100644 --- a/railties/lib/rails/test_unit/reporter.rb +++ b/railties/lib/rails/test_unit/reporter.rb @@ -18,7 +18,7 @@ module Rails if output_inline? && result.failure && (!result.skipped? || options[:verbose]) io.puts io.puts - io.puts format_failures(result).map { |line| color_output(line, by: result) } + io.puts color_output(result, by: result) io.puts io.puts format_rerun_snippet(result) io.puts @@ -66,12 +66,6 @@ module Rails "%s#%s = %.2f s = %s" % [result.class, result.name, result.time, result.result_code] end - def format_failures(result) - result.failures.map do |failure| - "#{failure.result_label}:\n#{result.location}:\n#{failure.message}\n" - end - end - def format_rerun_snippet(result) location, line = result.method(result.name).source_location "#{self.executable} #{relative_path_for(location)}:#{line}" diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 383f485db5..1c7d1e1f5f 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -348,6 +348,17 @@ module ApplicationTests end end + test "In production mode, STDOUT logging is enabled when RAILS_LOG_TO_STDOUT is set" do + restore_default_config + + with_rails_env "production" do + switch_env "RAILS_LOG_TO_STDOUT", "1" do + app 'production' + assert ActiveSupport::Logger.logger_outputs_to?(app.config.logger, STDOUT) + end + end + end + test "In production mode, config.public_file_server.enabled is disabled when RAILS_SERVE_STATIC_FILES is blank" do restore_default_config diff --git a/railties/test/application/initializers/frameworks_test.rb b/railties/test/application/initializers/frameworks_test.rb index 4c06b6324c..44209a52f7 100644 --- a/railties/test/application/initializers/frameworks_test.rb +++ b/railties/test/application/initializers/frameworks_test.rb @@ -16,7 +16,7 @@ module ApplicationTests # AC & AM test "set load paths set only if action controller or action mailer are in use" do - assert_nothing_raised NameError do + assert_nothing_raised do add_to_config <<-RUBY config.root = "#{app_path}" RUBY diff --git a/railties/test/application/loading_test.rb b/railties/test/application/loading_test.rb index 40abaf860d..efb21ae473 100644 --- a/railties/test/application/loading_test.rb +++ b/railties/test/application/loading_test.rb @@ -56,10 +56,10 @@ class LoadingTest < ActiveSupport::TestCase require "#{rails_root}/config/environment" - assert_nothing_raised(NameError) { Trackable } - assert_nothing_raised(NameError) { EmailLoggable } - assert_nothing_raised(NameError) { Orderable } - assert_nothing_raised(NameError) { Matchable } + assert_nothing_raised { Trackable } + assert_nothing_raised { EmailLoggable } + assert_nothing_raised { Orderable } + assert_nothing_raised { Matchable } end test "models without table do not panic on scope definitions when loaded" do diff --git a/railties/test/application/middleware/exceptions_test.rb b/railties/test/application/middleware/exceptions_test.rb index 7b4babb13b..639b01b562 100644 --- a/railties/test/application/middleware/exceptions_test.rb +++ b/railties/test/application/middleware/exceptions_test.rb @@ -85,7 +85,7 @@ module ApplicationTests test "unspecified route when action_dispatch.show_exceptions is set shows 404" do app.config.action_dispatch.show_exceptions = true - assert_nothing_raised(ActionController::RoutingError) do + assert_nothing_raised do get '/foo' assert_match "The page you were looking for doesn't exist.", last_response.body end @@ -95,7 +95,7 @@ module ApplicationTests app.config.action_dispatch.show_exceptions = true app.config.consider_all_requests_local = true - assert_nothing_raised(ActionController::RoutingError) do + assert_nothing_raised do get '/foo' assert_match "No route matches", last_response.body end diff --git a/railties/test/application/middleware/remote_ip_test.rb b/railties/test/application/middleware/remote_ip_test.rb index 97d5b5c698..37bd8a25c1 100644 --- a/railties/test/application/middleware/remote_ip_test.rb +++ b/railties/test/application/middleware/remote_ip_test.rb @@ -36,10 +36,10 @@ module ApplicationTests test "works with both headers individually" do make_basic_app - assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do + assert_nothing_raised do assert_equal "1.1.1.1", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1") end - assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do + assert_nothing_raised do assert_equal "1.1.1.2", remote_ip("HTTP_CLIENT_IP" => "1.1.1.2") end end @@ -49,7 +49,7 @@ module ApplicationTests app.config.action_dispatch.ip_spoofing_check = false end - assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do + assert_nothing_raised do assert_equal "1.1.1.1", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") end end diff --git a/railties/test/application/per_request_digest_cache_test.rb b/railties/test/application/per_request_digest_cache_test.rb index 210646c7c0..dfe3fc9354 100644 --- a/railties/test/application/per_request_digest_cache_test.rb +++ b/railties/test/application/per_request_digest_cache_test.rb @@ -29,6 +29,8 @@ class PerRequestDigestCacheTest < ActiveSupport::TestCase app_file 'app/controllers/customers_controller.rb', <<-RUBY class CustomersController < ApplicationController + self.perform_caching = true + def index render [ Customer.new('david', 1), Customer.new('dingus', 2) ] end diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index b0f348f3f1..a1735db5b3 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -502,6 +502,32 @@ module ApplicationTests assert_match '1 runs, 1 assertions', output end + def test_rails_db_create_all_restores_db_connection + create_test_file :models, 'account' + output = Dir.chdir(app_path) { `bin/rails db:create:all db:migrate && echo ".tables" | rails dbconsole` } + assert_match "ar_internal_metadata", output, "tables should be dumped" + end + + def test_rails_db_create_all_restores_db_connection_after_drop + create_test_file :models, 'account' + Dir.chdir(app_path) { `bin/rails db:create:all` } # create all to avoid warnings + output = Dir.chdir(app_path) { `bin/rails db:drop:all db:create:all db:migrate && echo ".tables" | rails dbconsole` } + assert_match "ar_internal_metadata", output, "tables should be dumped" + end + + def test_rake_passes_TESTOPTS_to_minitest + create_test_file :models, 'account' + output = Dir.chdir(app_path) { `bin/rake test TESTOPTS=-v` } + assert_match "AccountTest#test_truth", output, "passing TEST= should run selected test" + end + + def test_rake_passes_multiple_TESTOPTS_to_minitest + create_test_file :models, 'account' + output = Dir.chdir(app_path) { `bin/rake test TESTOPTS='-v --seed=1234'` } + assert_match "AccountTest#test_truth", output, "passing TEST= should run selected test" + assert_match "seed=1234", output, "passing TEST= should run selected test" + end + private def run_test_command(arguments = 'test/unit/test_test.rb') Dir.chdir(app_path) { `bin/rails t #{arguments}` } diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 921a5b36b5..c2c3638f07 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -64,8 +64,8 @@ class AppGeneratorTest < Rails::Generators::TestCase def test_assets run_generator - assert_file("app/views/layouts/application.html.erb", /stylesheet_link_tag\s+'application', media: 'all', 'data-turbolinks-track' => true/) - assert_file("app/views/layouts/application.html.erb", /javascript_include_tag\s+'application', 'data-turbolinks-track' => true/) + assert_file("app/views/layouts/application.html.erb", /stylesheet_link_tag\s+'application', media: 'all', 'data-turbolinks-track' => 'reload'/) + assert_file("app/views/layouts/application.html.erb", /javascript_include_tag\s+'application', 'data-turbolinks-track' => 'reload'/) assert_file("app/assets/stylesheets/application.css") assert_file("app/assets/javascripts/application.js") end diff --git a/railties/test/generators/channel_generator_test.rb b/railties/test/generators/channel_generator_test.rb index c1f0c03fbf..e31736a74c 100644 --- a/railties/test/generators/channel_generator_test.rb +++ b/railties/test/generators/channel_generator_test.rb @@ -5,6 +5,18 @@ class ChannelGeneratorTest < Rails::Generators::TestCase include GeneratorsTestHelper tests Rails::Generators::ChannelGenerator + def test_application_cable_skeleton_is_created + run_generator ['books'] + + assert_file "app/channels/application_cable/channel.rb" do |cable| + assert_match(/module ApplicationCable\n class Channel < ActionCable::Channel::Base\n/, cable) + end + + assert_file "app/channels/application_cable/connection.rb" do |cable| + assert_match(/module ApplicationCable\n class Connection < ActionCable::Connection::Base\n/, cable) + end + end + def test_channel_is_created run_generator ['chat'] diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index c8c8f0aa3b..ed6846abc3 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -6,6 +6,14 @@ class ModelGeneratorTest < Rails::Generators::TestCase include GeneratorsTestHelper arguments %w(Account name:string age:integer) + def test_application_record_skeleton_is_created + run_generator + assert_file "app/models/application_record.rb" do |record| + assert_match(/class ApplicationRecord < ActiveRecord::Base/, record) + assert_match(/self.abstract_class = true/, record) + end + end + def test_help_shows_invoked_generators_options content = run_generator ["--help"] assert_match(/ActiveRecord options:/, content) |