diff options
184 files changed, 2172 insertions, 1029 deletions
diff --git a/.github/issue_template.md b/.github/issue_template.md new file mode 100644 index 0000000000..2d071d4a71 --- /dev/null +++ b/.github/issue_template.md @@ -0,0 +1,15 @@ +### Steps to reproduce + +(Guidelines for creating a bug report are [available +here](http://guides.rubyonrails.org/contributing_to_ruby_on_rails.html#creating-a-bug-report)) + +### Expected behavior +Tell us what should happen + +### Actual behavior +Tell us what happens instead + +### System configuration +**Rails version**: + +**Ruby version**: diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000000..48f7b0e214 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,18 @@ +### Summary + +Provide a general description of the code changes in your pull +request... were there any bugs you had fixed? If so, mention them. If +these bugs have open GitHub issues, be sure to tag them here as well, +to keep the conversation linked together. + +### Other Information + +If there's anything else that's important and relevant to your pull +request, mention that information here. This could include +benchmarks, or other information. + +Finally, if your pull request affects documentation or any non-code +changes, guidelines for those changes are [available +here](http://guides.rubyonrails.org/contributing_to_ruby_on_rails.html#contributing-to-the-rails-documentation) + +Thanks for contributing to Rails! diff --git a/Gemfile.lock b/Gemfile.lock index d12aa0772f..5d0e815f86 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -28,66 +28,66 @@ 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 remote: . specs: - actioncable (5.0.0.beta2) - actionpack (= 5.0.0.beta2) + actioncable (5.0.0.beta3) + actionpack (= 5.0.0.beta3) nio4r (~> 1.2) websocket-driver (~> 0.6.1) - actionmailer (5.0.0.beta2) - actionpack (= 5.0.0.beta2) - actionview (= 5.0.0.beta2) - activejob (= 5.0.0.beta2) + actionmailer (5.0.0.beta3) + actionpack (= 5.0.0.beta3) + actionview (= 5.0.0.beta3) + activejob (= 5.0.0.beta3) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 1.0, >= 1.0.5) - actionpack (5.0.0.beta2) - actionview (= 5.0.0.beta2) - activesupport (= 5.0.0.beta2) + actionpack (5.0.0.beta3) + actionview (= 5.0.0.beta3) + activesupport (= 5.0.0.beta3) rack (~> 2.x) rack-test (~> 0.6.3) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.0.0.beta2) - activesupport (= 5.0.0.beta2) + actionview (5.0.0.beta3) + activesupport (= 5.0.0.beta3) builder (~> 3.1) erubis (~> 2.7.0) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.2) - activejob (5.0.0.beta2) - activesupport (= 5.0.0.beta2) + activejob (5.0.0.beta3) + activesupport (= 5.0.0.beta3) globalid (>= 0.3.6) - activemodel (5.0.0.beta2) - activesupport (= 5.0.0.beta2) - activerecord (5.0.0.beta2) - activemodel (= 5.0.0.beta2) - activesupport (= 5.0.0.beta2) + activemodel (5.0.0.beta3) + activesupport (= 5.0.0.beta3) + activerecord (5.0.0.beta3) + activemodel (= 5.0.0.beta3) + activesupport (= 5.0.0.beta3) arel (~> 7.0) - activesupport (5.0.0.beta2) + activesupport (5.0.0.beta3) concurrent-ruby (~> 1.0) i18n (~> 0.7) minitest (~> 5.1) tzinfo (~> 1.1) - rails (5.0.0.beta2) - actioncable (= 5.0.0.beta2) - actionmailer (= 5.0.0.beta2) - actionpack (= 5.0.0.beta2) - actionview (= 5.0.0.beta2) - activejob (= 5.0.0.beta2) - activemodel (= 5.0.0.beta2) - activerecord (= 5.0.0.beta2) - activesupport (= 5.0.0.beta2) + rails (5.0.0.beta3) + actioncable (= 5.0.0.beta3) + actionmailer (= 5.0.0.beta3) + actionpack (= 5.0.0.beta3) + actionview (= 5.0.0.beta3) + activejob (= 5.0.0.beta3) + activemodel (= 5.0.0.beta3) + activerecord (= 5.0.0.beta3) + activesupport (= 5.0.0.beta3) bundler (>= 1.3.0, < 2.0) - railties (= 5.0.0.beta2) + railties (= 5.0.0.beta3) sprockets-rails (>= 2.0.0) - railties (5.0.0.beta2) - actionpack (= 5.0.0.beta2) - activesupport (= 5.0.0.beta2) + railties (5.0.0.beta3) + actionpack (= 5.0.0.beta3) + activesupport (= 5.0.0.beta3) method_source rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) @@ -158,7 +158,7 @@ GEM mime-types (>= 1.16, < 3) metaclass (0.0.4) method_source (0.8.2) - mime-types (2.99) + mime-types (2.99.1) mini_portile2 (2.0.0) minitest (5.3.3) mocha (0.14.0) @@ -239,7 +239,7 @@ GEM sprockets (3.5.2) concurrent-ruby (~> 1.0) rack (> 1, < 3) - sprockets-rails (3.0.1) + sprockets-rails (3.0.2) actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) @@ -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/RAILS_VERSION b/RAILS_VERSION index 60b8d0bf66..d727b28ee9 100644 --- a/RAILS_VERSION +++ b/RAILS_VERSION @@ -1 +1 @@ -5.0.0.beta2 +5.0.0.beta3 @@ -1,4 +1,4 @@ -## Welcome to Rails +# Welcome to Rails Rails is a web-application framework that includes everything needed to create database-backed web applications according to the diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md index bfc229d795..e2e6819f98 100644 --- a/actioncable/CHANGELOG.md +++ b/actioncable/CHANGELOG.md @@ -1,7 +1,10 @@ -* Added ActionCable::SubscriptionAdapter::EventedRedis.em_redis_connector/redis_connector and - ActionCable::SubscriptionAdapter::Redis.redis_connector factory methods for redis connections, - so you can overwrite with your own initializers. This is used when you want to use different-than-standard Redis adapters, - like for Makara distributed Redis. +## Rails 5.0.0.beta3 (February 24, 2016) ## + +* Added `em_redis_connector` and `redis_connector` to + `ActionCable::SubscriptionAdapter::EventedRedis` and added `redis_connector` + to `ActionCable::SubscriptionAdapter::Redis`, so you can overwrite with your + own initializers. This is used when you want to use different-than-standard + Redis adapters, like for Makara distributed Redis. *DHH* 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..6a8b4eeb85 100644 --- a/actioncable/app/assets/javascripts/action_cable.coffee.erb +++ b/actioncable/app/assets/javascripts/action_cable.coffee.erb @@ -9,7 +9,7 @@ getConfig: (name) -> element = document.head.querySelector("meta[name='action-cable-#{name}']") - element?.getAttribute("content") + element?.getAttribute("content") ? '/cable' createWebSocketURL: (url) -> if url and not /^wss?:/i.test(url) @@ -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..4244322a1e 100644 --- a/actioncable/app/assets/javascripts/action_cable/connection.coffee +++ b/actioncable/app/assets/javascripts/action_cable/connection.coffee @@ -6,9 +6,11 @@ class ActionCable.Connection @reopenDelay: 500 constructor: (@consumer) -> - @open() send: (data) -> + unless @isOpen() + @open() + if @isOpen() @webSocket.send(JSON.stringify(data)) true @@ -16,9 +18,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 +32,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 +65,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 +83,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/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/client_socket.rb b/actioncable/lib/action_cable/connection/client_socket.rb index 95e1ac4c16..f6b11e93f0 100644 --- a/actioncable/lib/action_cable/connection/client_socket.rb +++ b/actioncable/lib/action_cable/connection/client_socket.rb @@ -50,14 +50,16 @@ module ActionCable @driver.on(:error) { |e| emit_error(e.message) } @stream = ActionCable::Connection::Stream.new(@stream_event_loop, self) + end + + def start_driver + return if @driver.nil? || @driver_started + @stream.hijack_rack_socket if callback = @env['async.callback'] callback.call([101, {}, @stream]) end - end - def start_driver - return if @driver.nil? || @driver_started @driver_started = true @driver.start end 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/stream.rb b/actioncable/lib/action_cable/connection/stream.rb index ace250cd16..2d97b28c09 100644 --- a/actioncable/lib/action_cable/connection/stream.rb +++ b/actioncable/lib/action_cable/connection/stream.rb @@ -4,15 +4,13 @@ module ActionCable # This class is heavily based on faye-websocket-ruby # # Copyright (c) 2010-2015 James Coglan - class Stream + class Stream # :nodoc: def initialize(event_loop, socket) @event_loop = event_loop @socket_object = socket @stream_send = socket.env['stream.send'] @rack_hijack_io = nil - - hijack_rack_socket end def each(&callback) @@ -39,16 +37,16 @@ module ActionCable @socket_object.parse(data) end - private - def hijack_rack_socket - return unless @socket_object.env['rack.hijack'] + def hijack_rack_socket + return unless @socket_object.env['rack.hijack'] - @socket_object.env['rack.hijack'].call - @rack_hijack_io = @socket_object.env['rack.hijack_io'] + @socket_object.env['rack.hijack'].call + @rack_hijack_io = @socket_object.env['rack.hijack_io'] - @event_loop.attach(@rack_hijack_io, self) - end + @event_loop.attach(@rack_hijack_io, self) + end + private def clean_rack_hijack return unless @rack_hijack_io @event_loop.detach(@rack_hijack_io, self) 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/engine.rb b/actioncable/lib/action_cable/engine.rb index f5f1cb59e0..ae0c59dccd 100644 --- a/actioncable/lib/action_cable/engine.rb +++ b/actioncable/lib/action_cable/engine.rb @@ -6,7 +6,7 @@ require "active_support/core_ext/hash/indifferent_access" module ActionCable class Railtie < Rails::Engine # :nodoc: config.action_cable = ActiveSupport::OrderedOptions.new - config.action_cable.url = '/cable' + config.action_cable.mount_path = '/cable' config.eager_load_namespaces << ActionCable @@ -40,5 +40,16 @@ module ActionCable options.each { |k,v| send("#{k}=", v) } end end + + initializer "action_cable.routes" do + config.after_initialize do |app| + config = app.config + unless config.action_cable.mount_path.nil? + app.routes.prepend do + mount ActionCable.server => config.action_cable.mount_path, internal: true + end + end + end + end end end diff --git a/actioncable/lib/action_cable/gem_version.rb b/actioncable/lib/action_cable/gem_version.rb index a71603e61a..67adeefaff 100644 --- a/actioncable/lib/action_cable/gem_version.rb +++ b/actioncable/lib/action_cable/gem_version.rb @@ -8,7 +8,7 @@ module ActionCable MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/actioncable/lib/action_cable/helpers/action_cable_helper.rb b/actioncable/lib/action_cable/helpers/action_cable_helper.rb index b82751468a..200732fdcd 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: # @@ -20,9 +20,20 @@ module ActionCable # Make sure to specify the correct server location in each of your environments # config file: # - # config.action_cable.url = "ws://example.com:28080" + # config.action_cable.mount_path = "/cable123" + # <%= action_cable_meta_tag %> would render: + # => <meta name="action-cable-url" content="/cable123" /> + # + # config.action_cable.url = "ws://actioncable.com" + # <%= action_cable_meta_tag %> would render: + # => <meta name="action-cable-url" content="ws://actioncable.com" /> + # def action_cable_meta_tag - tag "meta", name: "action-cable-url", content: Rails.application.config.action_cable.url + tag "meta", name: "action-cable-url", content: ( + ActionCable.server.config.url || + ActionCable.server.config.mount_path || + raise("No Action Cable URL configured -- please configure this at config.action_cable.url") + ) end end end 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..9a7301287c 100644 --- a/actioncable/lib/action_cable/server/configuration.rb +++ b/actioncable/lib/action_cable/server/configuration.rb @@ -1,12 +1,12 @@ 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 attr_accessor :connection_class, :worker_pool_size attr_accessor :disable_request_forgery_protection, :allowed_request_origins - attr_accessor :cable, :url + attr_accessor :cable, :url, :mount_path attr_accessor :channel_paths # :nodoc: 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..fb11f9be64 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 @@ -108,6 +108,26 @@ class ActionCable::Connection::BaseTest < ActionCable::TestCase end end + test "rejecting a connection causes a 404" do + run_in_eventmachine do + class CallMeMaybe + def call(*) + raise 'Do not call me!' + end + end + + env = Rack::MockRequest.env_for( + "/test", + { 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket', + 'HTTP_ORIGIN' => 'http://rubyonrails.org', 'rack.hijack' => CallMeMaybe.new } + ) + + connection = ActionCable::Connection::Base.new(@server, env) + response = connection.process + assert_equal 404, response[0] + end + end + private def open_connection env = Rack::MockRequest.env_for "/test", 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket', 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..604e332dad 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,11 @@ +## Rails 5.0.0.beta3 (February 24, 2016) ## + +* 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/gem_version.rb b/actionmailer/lib/action_mailer/gem_version.rb index a1ee5fb238..cbe5fc3e64 100644 --- a/actionmailer/lib/action_mailer/gem_version.rb +++ b/actionmailer/lib/action_mailer/gem_version.rb @@ -8,7 +8,7 @@ module ActionMailer MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") 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..6b73b29ace 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,34 @@ +* Update default rendering policies when the controller action did + not explicitly indicate a response. + + For API controllers, the implicit render always renders "204 No Content" + and does not account for any templates. + + For other controllers, the following conditions are checked: + + First, if a template exists for the controller action, it is rendered. + This template lookup takes into account the action name, locales, format, + variant, template handlers, etc. (see +render+ for details). + + Second, if other templates exist for the controller action but is not in + the right format (or variant, etc.), an <tt>ActionController::UnknownFormat</tt> + is raised. The list of available templates is assumed to be a complete + enumeration of all the possible formats (or variants, etc.); that is, + having only HTML and JSON templates indicate that the controller action is + not meant to handle XML requests. + + Third, if the current request is an "interactive" browser request (the user + navigated here by entering the URL in the address bar, submiting a form, + clicking on a link, etc. as opposed to an XHR or non-browser API request), + <tt>ActionView::UnknownFormat</tt> is raised to display a helpful error + message. + + Finally, it falls back to the same "204 No Content" behavior as API controllers. + + *Godfrey Chan*, *Jon Moss*, *Kasper Timm Hansen*, *Mike Clark*, *Matthew Draper* + +## Rails 5.0.0.beta3 (February 24, 2016) ## + * Add application/gzip as a default mime type. *Mehmet Emin İNAÇ* @@ -37,13 +68,13 @@ end end ``` - + Passing `as: :json` to integration test request helpers will set the format, content type and encode the parameters as JSON. - + Then on the response side, `parsed_body` will parse the body according to the content type the response has. - + Currently JSON is the only supported MIME type. Add your own with `ActionDispatch::IntegrationTest.register_encoder`. 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/basic_implicit_render.rb b/actionpack/lib/action_controller/metal/basic_implicit_render.rb index 6c6f8381ff..cef65a362c 100644 --- a/actionpack/lib/action_controller/metal/basic_implicit_render.rb +++ b/actionpack/lib/action_controller/metal/basic_implicit_render.rb @@ -1,5 +1,5 @@ module ActionController - module BasicImplicitRender + module BasicImplicitRender # :nodoc: def send_action(method, *args) super.tap { default_render unless performed? } end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index 17fcc2fa02..6b540d42c7 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -1,29 +1,80 @@ +require 'active_support/core_ext/string/strip' + module ActionController + # Handles implicit rendering for a controller action when it did not + # explicitly indicate an appropiate response via methods such as +render+, + # +respond_to+, +redirect+ or +head+. + # + # For API controllers, the implicit render always renders "204 No Content" + # and does not account for any templates. + # + # For other controllers, the following conditions are checked: + # + # First, if a template exists for the controller action, it is rendered. + # This template lookup takes into account the action name, locales, format, + # variant, template handlers, etc. (see +render+ for details). + # + # Second, if other templates exist for the controller action but is not in + # the right format (or variant, etc.), an <tt>ActionController::UnknownFormat</tt> + # is raised. The list of available templates is assumed to be a complete + # enumeration of all the possible formats (or variants, etc.); that is, + # having only HTML and JSON templates indicate that the controller action is + # not meant to handle XML requests. + # + # Third, if the current request is an "interactive" browser request (the user + # navigated here by entering the URL in the address bar, submiting a form, + # clicking on a link, etc. as opposed to an XHR or non-browser API request), + # <tt>ActionView::UnknownFormat</tt> is raised to display a helpful error + # message. + # + # Finally, it falls back to the same "204 No Content" behavior as API controllers. module ImplicitRender + # :stopdoc: include BasicImplicitRender - # Renders the template corresponding to the controller action, if it exists. - # The action name, format, and variant are all taken into account. - # For example, the "new" action with an HTML format and variant "phone" - # would try to render the <tt>new.html+phone.erb</tt> template. - # - # If no template is found <tt>ActionController::BasicImplicitRender</tt>'s implementation is called, unless - # a block is passed. In that case, it will override the super implementation. - # - # default_render do - # head 404 # No template was found - # end def default_render(*args) if template_exists?(action_name.to_s, _prefixes, variants: request.variant) render(*args) - else - if block_given? - yield(*args) - else - logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger - super + elsif any_templates?(action_name.to_s, _prefixes) + message = "#{self.class.name}\##{action_name} does not know how to respond " \ + "to this request. There are other templates available for this controller " \ + "action but none of them were suitable for this request.\n\n" \ + "This usually happens when the client requested an unsupported format " \ + "(e.g. requesting HTML content from a JSON endpoint or vice versa), but " \ + "it might also be failing due to other constraints, such as locales or" \ + "variants.\n" + + if request.formats.any? + message << "\nRequested format(s): #{request.formats.join(", ")}" end + + if request.variant.any? + message << "\nRequested variant(s): #{request.variant.join(", ")}" + end + + raise ActionController::UnknownFormat, message + elsif interactive_browser_request? + message = "You did not define any templates for #{self.class.name}\##{action_name}. " \ + "This is not necessarily a problem (e.g. you might be building an API endpoint " \ + "that does not require any templates), and the controller would usually respond " \ + "with `head :no_content` for your convenience.\n\n" \ + "However, you appear to have navigated here from an interactive browser request – " \ + "such as by navigating to this URL directly, clicking on a link or submitting a form. " \ + "Rendering a `head :no_content` in this case could have resulted in unexpected UI " \ + "behavior in the browser.\n\n" \ + "If you expected the `head :no_content` response, you do not need to take any " \ + "actions – requests coming from an XHR (AJAX) request or other non-browser clients " \ + "will receive the \"204 No Content\" response as expected.\n\n" \ + "If you did not expect this behavior, you can resolve this error by adding a " \ + "template for this controller action (usually `#{action_name}.html.erb`) or " \ + "otherwise indicate the appropriate response in the action using `render`, " \ + "`redirect_to`, `head`, etc.\n" + + raise ActionController::UnknownFormat, message + else + logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger + super end end @@ -32,5 +83,11 @@ module ActionController "default_render" end end + + private + + def interactive_browser_request? + request.format == Mime[:html] && !request.xhr? + end end end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 25ec3cf5b6..a01110d474 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -579,7 +579,7 @@ module ActionController end def inspect - "<#{self.class} #{@parameters}>" + "<#{self.class} #{@parameters} permitted: #{@permitted}>" end def method_missing(method_sym, *args, &block) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 0c4b661214..700317614f 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 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..cfd6681dd1 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 @@ -81,7 +82,7 @@ module ActionDispatch end def requirements # :nodoc: - # needed for rails `rake routes` + # needed for rails `rails routes` @defaults.merge(path.requirements).delete_if { |_,v| /.+?/ == v } 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/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 735b5939dd..711d8b016a 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -23,7 +23,7 @@ module ActionDispatch # preload lists is `18.weeks`. # * `subdomains`: Set to `true` to tell the browser to apply these settings # to all subdomains. This protects your cookies from interception by a - # vulnerable site on a subdomain. Defaults to `false`. + # vulnerable site on a subdomain. Defaults to `true`. # * `preload`: Advertise that this site may be included in browsers' # preloaded HSTS lists. HSTS protects your site on every visit *except the # first visit* since it hasn't seen your HSTS header yet. To close this @@ -49,7 +49,7 @@ module ActionDispatch if options[:host] || options[:port] ActiveSupport::Deprecation.warn <<-end_warning.strip_heredoc The `:host` and `:port` options are moving within `:redirect`: - `config.ssl_options = { redirect: { host: …, port: … }}`. + `config.ssl_options = { redirect: { host: …, port: … } }`. end_warning @redirect = options.slice(:host, :port) else @@ -57,6 +57,17 @@ module ActionDispatch end @secure_cookies = secure_cookies + + if hsts != true && hsts != false && hsts[:subdomains].nil? + hsts[:subdomains] = false + + ActiveSupport::Deprecation.warn <<-end_warning.strip_heredoc + In Rails 5.1, The `:subdomains` option of HSTS config will be treated as true if + unspecified. Set `config.ssl_options = { hsts: { subdomains: false } }` to opt out + of this behavior. + end_warning + end + @hsts_header = build_hsts_header(normalize_hsts_options(hsts)) end diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 983f1daeb3..5d30a545a2 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? @@ -51,7 +51,7 @@ module ActionDispatch ## # This class is just used for displaying route information when someone - # executes `rake routes` or looks at the RoutingError page. + # executes `rails routes` or looks at the RoutingError page. # People should not use this class. class RoutesInspector # :nodoc: def initialize(routes) 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/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 778c5482d3..157f401f54 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -8,7 +8,7 @@ module ActionPack MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end 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/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 76e2d3ff43..d0c7b2e06a 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -160,7 +160,14 @@ class RespondToController < ActionController::Base end end - def variant_with_implicit_rendering + def variant_with_implicit_template_rendering + # This has exactly one variant template defined in the file system (+mobile.html.erb), + # which raises the regular MissingTemplate error for other variants. + end + + def variant_without_implicit_template_rendering + # This differs from the above in that it does not have any templates defined in the file + # system, which triggers the ImplicitRender (204 No Content) behavior. end def variant_with_format_and_custom_render @@ -272,6 +279,8 @@ class RespondToController < ActionController::Base end class RespondToControllerTest < ActionController::TestCase + NO_CONTENT_WARNING = "No template found for RespondToController#variant_without_implicit_template_rendering, rendering head :no_content" + def setup super @request.host = "www.example.com" @@ -616,30 +625,69 @@ class RespondToControllerTest < ActionController::TestCase end def test_invalid_variant + assert_raises(ActionController::UnknownFormat) do + get :variant_with_implicit_template_rendering, params: { v: :invalid } + end + end + + def test_variant_not_set_regular_unknown_format + assert_raises(ActionController::UnknownFormat) do + get :variant_with_implicit_template_rendering + end + end + + def test_variant_with_implicit_template_rendering + get :variant_with_implicit_template_rendering, params: { v: :mobile } + assert_equal "text/html", @response.content_type + assert_equal "mobile", @response.body + end + + def test_variant_without_implicit_rendering_from_browser + assert_raises(ActionController::UnknownFormat) do + get :variant_without_implicit_template_rendering, params: { v: :does_not_matter } + end + end + + def test_variant_variant_not_set_and_without_implicit_rendering_from_browser + assert_raises(ActionController::UnknownFormat) do + get :variant_without_implicit_template_rendering + end + end + + def test_variant_without_implicit_rendering_from_xhr logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new old_logger, ActionController::Base.logger = ActionController::Base.logger, logger - get :variant_with_implicit_rendering, params: { v: :invalid } + get :variant_without_implicit_template_rendering, xhr: true, params: { v: :does_not_matter } assert_response :no_content - assert_equal 1, logger.logged(:info).select{ |s| s =~ /No template found/ }.size, "Implicit head :no_content not logged" + + assert_equal 1, logger.logged(:info).select{ |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged" ensure ActionController::Base.logger = old_logger end - def test_variant_not_set_regular_template_missing - get :variant_with_implicit_rendering + def test_variant_without_implicit_rendering_from_api + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + old_logger, ActionController::Base.logger = ActionController::Base.logger, logger + + get :variant_without_implicit_template_rendering, format: 'json', params: { v: :does_not_matter } assert_response :no_content + + assert_equal 1, logger.logged(:info).select{ |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged" + ensure + ActionController::Base.logger = old_logger end - def test_variant_with_implicit_rendering - get :variant_with_implicit_rendering, params: { v: :implicit } + def test_variant_variant_not_set_and_without_implicit_rendering_from_xhr + logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new + old_logger, ActionController::Base.logger = ActionController::Base.logger, logger + + get :variant_without_implicit_template_rendering, xhr: true assert_response :no_content - end - def test_variant_with_implicit_template_rendering - get :variant_with_implicit_rendering, params: { v: :mobile } - assert_equal "text/html", @response.content_type - assert_equal "mobile", @response.body + assert_equal 1, logger.logged(:info).select { |s| s == NO_CONTENT_WARNING }.size, "Implicit head :no_content not logged" + ensure + ActionController::Base.logger = old_logger end def test_variant_with_format_and_custom_render @@ -778,24 +826,3 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "phone", @response.body end end - -class RespondToWithBlockOnDefaultRenderController < ActionController::Base - def show - default_render do - render body: 'default_render yielded' - end - end -end - -class RespondToWithBlockOnDefaultRenderControllerTest < ActionController::TestCase - def setup - super - @request.host = "www.example.com" - end - - def test_default_render_uses_block_when_no_template_exists - get :show - assert_equal "default_render yielded", @response.body - assert_equal "text/plain", @response.content_type - end -end diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index 4ef5bed30d..cea265f9ab 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -4,6 +4,8 @@ require 'active_support/core_ext/hash/transform_values' class ParametersAccessorsTest < ActiveSupport::TestCase setup do + ActionController::Parameters.permit_all_parameters = false + @params = ActionController::Parameters.new( person: { age: '32', @@ -176,12 +178,20 @@ class ParametersAccessorsTest < ActiveSupport::TestCase assert(@params != false) end - test "inspect shows both class name and parameters" do + test "inspect shows both class name, parameters and permitted flag" do assert_equal( '<ActionController::Parameters {"person"=>{"age"=>"32", '\ - '"name"=>{"first"=>"David", "last"=>"Heinemeier Hansson"}, ' \ - '"addresses"=>[{"city"=>"Chicago", "state"=>"Illinois"}]}}>', + '"name"=>{"first"=>"David", "last"=>"Heinemeier Hansson"}, ' \ + '"addresses"=>[{"city"=>"Chicago", "state"=>"Illinois"}]}} permitted: false>', @params.inspect ) end + + test "inspect prints updated permitted flag in the output" do + assert_match(/permitted: false/, @params.inspect) + + @params.permit! + + assert_match(/permitted: true/, @params.inspect) + end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 60c6518c62..83d7405e4d 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -26,6 +26,9 @@ end class ImplicitRenderTestController < ActionController::Base def empty_action end + + def empty_action_with_template + end end class TestController < ActionController::Base @@ -537,10 +540,28 @@ end class ImplicitRenderTest < ActionController::TestCase tests ImplicitRenderTestController - def test_implicit_no_content_response - get :empty_action + def test_implicit_no_content_response_as_browser + assert_raises(ActionController::UnknownFormat) do + get :empty_action + end + end + + def test_implicit_no_content_response_as_xhr + get :empty_action, xhr: true assert_response :no_content end + + def test_implicit_success_response_with_right_format + get :empty_action_with_template + assert_equal "<h1>Empty action rendered this implicitly.</h1>\n", @response.body + assert_response :success + end + + def test_implicit_unknown_format_response + assert_raises(ActionController::UnknownFormat) do + get :empty_action_with_template, format: 'json' + end + end end class HeadRenderTest < ActionController::TestCase diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index c645af88d7..f7dcbc1984 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -133,11 +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] || :post) %>" + render inline: "<%= button_to 'Button', (params[:form_path] || '/per_form_tokens/post_one'), method: params[:form_method] %>" end def post_one @@ -710,6 +710,20 @@ class PerFormTokensControllerTest < ActionController::TestCase 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 } 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/response_test.rb b/actionpack/test/dispatch/response_test.rb index 8b3849cb7a..cd385982d9 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -445,4 +445,12 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_equal('application/xml; charset=utf-16', @response.headers['Content-Type']) end + + test "we can set strong ETag by directly adding it as header" do + @response = ActionDispatch::Response.create + @response.add_header "ETag", '"202cb962ac59075b964b07152d234b70"' + + assert_equal('"202cb962ac59075b964b07152d234b70"', @response.etag) + assert_equal('"202cb962ac59075b964b07152d234b70"', @response.headers['ETag']) + end end 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/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index c66a0e6a7a..18ff894b31 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -7,7 +7,7 @@ class SSLTest < ActionDispatch::IntegrationTest def build_app(headers: {}, ssl_options: {}) headers = HEADERS.merge(headers) - ActionDispatch::SSL.new lambda { |env| [200, headers, []] }, ssl_options + ActionDispatch::SSL.new lambda { |env| [200, headers, []] }, ssl_options.reverse_merge(hsts: { subdomains: true }) end end @@ -98,15 +98,16 @@ end class StrictTransportSecurityTest < SSLTest EXPECTED = 'max-age=15552000' + EXPECTED_WITH_SUBDOMAINS = 'max-age=15552000; includeSubDomains' - def assert_hsts(expected, url: 'https://example.org', hsts: {}, headers: {}) + def assert_hsts(expected, url: 'https://example.org', hsts: { subdomains: true }, headers: {}) self.app = build_app ssl_options: { hsts: hsts }, headers: headers get url assert_equal expected, response.headers['Strict-Transport-Security'] end test 'enabled by default' do - assert_hsts EXPECTED + assert_hsts EXPECTED_WITH_SUBDOMAINS end test 'not sent with http:// responses' do @@ -126,11 +127,15 @@ class StrictTransportSecurityTest < SSLTest end test ':expires sets max-age' do - assert_hsts 'max-age=500', hsts: { expires: 500 } + assert_deprecated do + assert_hsts 'max-age=500', hsts: { expires: 500 } + end end test ':expires supports AS::Duration arguments' do - assert_hsts 'max-age=31557600', hsts: { expires: 1.year } + assert_deprecated do + assert_hsts 'max-age=31557600', hsts: { expires: 1.year } + end end test 'include subdomains' do @@ -142,11 +147,15 @@ class StrictTransportSecurityTest < SSLTest end test 'opt in to browser preload lists' do - assert_hsts "#{EXPECTED}; preload", hsts: { preload: true } + assert_deprecated do + assert_hsts "#{EXPECTED}; preload", hsts: { preload: true } + end end test 'opt out of browser preload lists' do - assert_hsts EXPECTED, hsts: { preload: false } + assert_deprecated do + assert_hsts EXPECTED, hsts: { preload: false } + end end end diff --git a/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb b/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb new file mode 100644 index 0000000000..ded99ba52d --- /dev/null +++ b/actionpack/test/fixtures/implicit_render_test/empty_action_with_mobile_variant.html+mobile.erb @@ -0,0 +1 @@ +mobile diff --git a/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb b/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb new file mode 100644 index 0000000000..dd294f8cf6 --- /dev/null +++ b/actionpack/test/fixtures/implicit_render_test/empty_action_with_template.html.erb @@ -0,0 +1 @@ +<h1>Empty action rendered this implicitly.</h1> diff --git a/actionpack/test/fixtures/respond_to/variant_with_implicit_rendering.html+mobile.erb b/actionpack/test/fixtures/respond_to/variant_with_implicit_template_rendering.html+mobile.erb index 317801ad30..317801ad30 100644 --- a/actionpack/test/fixtures/respond_to/variant_with_implicit_rendering.html+mobile.erb +++ b/actionpack/test/fixtures/respond_to/variant_with_implicit_template_rendering.html+mobile.erb diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index bd7ce14e04..d084f2b1e0 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,30 @@ +## Rails 5.0.0.beta3 (February 24, 2016) ## + +* Collection rendering can cache and fetch multiple partials at once. + + Collections rendered as: + + ```ruby + <%= render partial: 'notifications/notification', collection: @notifications, as: :notification, cached: true %> + ``` + + 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 + <% cache notification do %> + <%# ... %> + <% end %> + ``` + + Then any collection renders shares that cache when attempting to read multiple + ones at once. + + *Kasper Timm Hansen* + * Add support for nested hashes/arrays to `:params` option of `button_to` helper. *James Coleman* @@ -197,31 +224,6 @@ *Ulisses Almeida* -* Collection rendering can cache and fetch multiple partials at once. - - Collections rendered as: - - ```ruby - <%= render partial: 'notifications/notification', collection: @notifications, as: :notification, cached: true %> - ``` - - 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 - <% cache notification do %> - <%# ... %> - <% 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 count layouts as dependencies for partials. diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index f3c5d6c8df..b99d1af998 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -4,7 +4,7 @@ require 'monitor' module ActionView class Digestor - @@digest_monitor = Monitor.new + @@digest_mutex = Mutex.new class PerRequestDigestCacheExpiry < Struct.new(:app) # :nodoc: def call(env) @@ -20,111 +20,104 @@ module ActionView # * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt> # * <tt>dependencies</tt> - An array of dependent views # * <tt>partial</tt> - Specifies whether the template is a partial - def digest(name:, finder:, **options) - options.assert_valid_keys(:dependencies, :partial) - - cache_key = ([ name ].compact + Array.wrap(options[:dependencies])).join('.') + def digest(name:, finder:, dependencies: []) + dependencies ||= [] + cache_key = ([ name ].compact + dependencies).join('.') # this is a correctly done double-checked locking idiom # (Concurrent::Map's lookups have volatile semantics) - finder.digest_cache[cache_key] || @@digest_monitor.synchronize do + finder.digest_cache[cache_key] || @@digest_mutex.synchronize do finder.digest_cache.fetch(cache_key) do # re-check under lock - compute_and_store_digest(cache_key, name, finder, options) + partial = name.include?("/_") + root = tree(name, finder, partial) + dependencies.each do |injected_dep| + root.children << Injected.new(injected_dep, nil, nil) + end + finder.digest_cache[cache_key] = root.digest(finder) end end end - private - def compute_and_store_digest(cache_key, name, finder, options) # called under @@digest_monitor lock - klass = if options[:partial] || name.include?("/_") - # Prevent re-entry or else recursive templates will blow the stack. - # There is no need to worry about other threads seeing the +false+ value, - # as they will then have to wait for this thread to let go of the @@digest_monitor lock. - pre_stored = finder.digest_cache.put_if_absent(cache_key, false).nil? # put_if_absent returns nil on insertion - PartialDigestor - else - Digestor - end - - finder.digest_cache[cache_key] = stored_digest = klass.new(name, finder, options).digest - ensure - # something went wrong or ActionView::Resolver.caching? is false, make sure not to corrupt the @@cache - finder.digest_cache.delete_pair(cache_key, false) if pre_stored && !stored_digest - end - end - - attr_reader :name, :finder, :options + def logger + ActionView::Base.logger || NullLogger + end - def initialize(name, finder, options = {}) - @name, @finder = name, finder - @options = options - end + # Create a dependency tree for template named +name+. + def tree(name, finder, partial = false, seen = {}) + logical_name = name.gsub(%r|/_|, "/") - def digest - Digest::MD5.hexdigest("#{source}-#{dependency_digest}").tap do |digest| - logger.debug " Cache digest for #{template.inspect}: #{digest}" - end - rescue ActionView::MissingTemplate - logger.error " Couldn't find template for digesting: #{name}" - '' - end + if finder.disable_cache { finder.exists?(logical_name, [], partial) } + template = finder.disable_cache { finder.find(logical_name, [], partial) } - def dependencies - DependencyTracker.find_dependencies(name, template, finder.view_paths) - rescue ActionView::MissingTemplate - logger.error " '#{name}' file doesn't exist, so no dependencies" - [] - end + if node = seen[template.identifier] # handle cycles in the tree + node + else + node = seen[template.identifier] = Node.create(name, logical_name, template, partial) - def nested_dependencies - dependencies.collect do |dependency| - dependencies = PartialDigestor.new(dependency, finder).nested_dependencies - dependencies.any? ? { dependency => dependencies } : dependency + deps = DependencyTracker.find_dependencies(name, template, finder.view_paths) + deps.uniq { |n| n.gsub(%r|/_|, "/") }.each do |dep_file| + node.children << tree(dep_file, finder, true, seen) + end + node + end + else + logger.error " '#{name}' file doesn't exist, so no dependencies" + logger.error " Couldn't find template for digesting: #{name}" + seen[name] ||= Missing.new(name, logical_name, nil) + end end end - private - class NullLogger - def self.debug(_); end - def self.error(_); end - end + class Node + attr_reader :name, :logical_name, :template, :children - def logger - ActionView::Base.logger || NullLogger + def self.create(name, logical_name, template, partial) + klass = partial ? Partial : Node + klass.new(name, logical_name, template, []) end - def logical_name - name.gsub(%r|/_|, "/") + def initialize(name, logical_name, template, children = []) + @name = name + @logical_name = logical_name + @template = template + @children = children end - def partial? - false + def digest(finder, stack = []) + Digest::MD5.hexdigest("#{template.source}-#{dependency_digest(finder, stack)}") end - def template - @template ||= finder.disable_cache { finder.find(logical_name, [], partial?) } + def dependency_digest(finder, stack) + children.map do |node| + if stack.include?(node) + false + else + finder.digest_cache[node.name] ||= begin + stack.push node + node.digest(finder, stack).tap { stack.pop } + end + end + end.join("-") end - def source - template.source + def to_dep_map + children.any? ? { name => children.map(&:to_dep_map) } : name end + end - def dependency_digest - template_digests = dependencies.collect do |template_name| - Digestor.digest(name: template_name, finder: finder, partial: true) - end + class Partial < Node; end - (template_digests + injected_dependencies).join("-") - end + class Missing < Node + def digest(finder, _ = []) '' end + end - def injected_dependencies - Array.wrap(options[:dependencies]) - end - end + class Injected < Node + def digest(finder, _ = []) name end + end - class PartialDigestor < Digestor # :nodoc: - def partial? - true + class NullLogger + def self.debug(_); end + def self.error(_); end end end end diff --git a/actionview/lib/action_view/gem_version.rb b/actionview/lib/action_view/gem_version.rb index bb5c96cb39..efb565bf59 100644 --- a/actionview/lib/action_view/gem_version.rb +++ b/actionview/lib/action_view/gem_version.rb @@ -8,7 +8,7 @@ module ActionView MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end 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 2454ed4ed4..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: { action: url, method: method }) + 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/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 86afedaa2d..626c4b8f5e 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -70,8 +70,6 @@ module ActionView @details_keys.clear end - def self.empty?; @details_keys.empty?; end - def self.digest_caches @details_keys.values.map(&:digest_cache) end @@ -138,6 +136,11 @@ module ActionView end alias :template_exists? :exists? + def any?(name, prefixes = [], partial = false) + @view_paths.exists?(*args_for_any(name, prefixes, partial)) + end + alias :any_templates? :any? + # Adds fallbacks to the view paths. Useful in cases when you are rendering # a :file. def with_fallbacks @@ -174,6 +177,32 @@ module ActionView [user_details, details_key] end + def args_for_any(name, prefixes, partial) # :nodoc: + name, prefixes = normalize_name(name, prefixes) + details, details_key = detail_args_for_any + [name, prefixes, partial || false, details, details_key] + end + + def detail_args_for_any # :nodoc: + @detail_args_for_any ||= begin + details = {} + + registered_details.each do |k| + if k == :variants + details[k] = :any + else + details[k] = Accessors::DEFAULT_PROCS[k].call + end + end + + if @cache + [details, DetailsKey.get(details)] + else + [details, nil] + end + end + end + # Support legacy foo.erb names even though we now ignore .erb # as well as incorrectly putting part of the path in the template # name instead of the prefix. diff --git a/actionview/lib/action_view/renderer/abstract_renderer.rb b/actionview/lib/action_view/renderer/abstract_renderer.rb index 23e672a95f..1dddf53df0 100644 --- a/actionview/lib/action_view/renderer/abstract_renderer.rb +++ b/actionview/lib/action_view/renderer/abstract_renderer.rb @@ -15,7 +15,7 @@ module ActionView # that new object is called in turn. This abstracts the setup and rendering # into a separate classes for partials and templates. class AbstractRenderer #:nodoc: - delegate :find_template, :find_file, :template_exists?, :with_fallbacks, :with_layout_format, :formats, :to => :@lookup_context + delegate :find_template, :find_file, :template_exists?, :any_templates?, :with_fallbacks, :with_layout_format, :formats, :to => :@lookup_context def initialize(lookup_context) @lookup_context = lookup_context diff --git a/actionview/lib/action_view/tasks/dependencies.rake b/actionview/lib/action_view/tasks/dependencies.rake index 9932ff8b6d..045bdf5691 100644 --- a/actionview/lib/action_view/tasks/dependencies.rake +++ b/actionview/lib/action_view/tasks/dependencies.rake @@ -2,13 +2,13 @@ namespace :cache_digests do desc 'Lookup nested dependencies for TEMPLATE (like messages/show or comments/_comment.html)' task :nested_dependencies => :environment do abort 'You must provide TEMPLATE for the task to run' unless ENV['TEMPLATE'].present? - puts JSON.pretty_generate ActionView::Digestor.new(CacheDigests.template_name, CacheDigests.finder).nested_dependencies + puts JSON.pretty_generate ActionView::Digestor.tree(CacheDigests.template_name, CacheDigests.finder).children.map(&:to_dep_map) end desc 'Lookup first-level dependencies for TEMPLATE (like messages/show or comments/_comment.html)' task :dependencies => :environment do abort 'You must provide TEMPLATE for the task to run' unless ENV['TEMPLATE'].present? - puts JSON.pretty_generate ActionView::Digestor.new(CacheDigests.template_name, CacheDigests.finder).dependencies + puts JSON.pretty_generate ActionView::Digestor.tree(CacheDigests.template_name, CacheDigests.finder).children.map(&:name) end class CacheDigests diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 8a675cd521..b6de0b03bf 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -222,7 +222,7 @@ module ActionView end def find_template_paths(query) - Dir[query].reject do |filename| + Dir[query].uniq.reject do |filename| File.directory?(filename) || # deals with case-insensitive file systems. !File.fnmatch(query, filename, File::FNM_EXTGLOB) @@ -340,7 +340,11 @@ module ActionView query = escape_entry(File.join(@path, path)) exts = EXTENSIONS.map do |ext, prefix| - "{#{details[ext].compact.uniq.map { |e| "#{prefix}#{e}," }.join}}" + if ext == :variants && details[ext] == :any + "{#{prefix}*,}" + else + "{#{details[ext].compact.uniq.map { |e| "#{prefix}#{e}," }.join}}" + end end.join query + exts diff --git a/actionview/lib/action_view/view_paths.rb b/actionview/lib/action_view/view_paths.rb index 37722013ce..b46fe06b01 100644 --- a/actionview/lib/action_view/view_paths.rb +++ b/actionview/lib/action_view/view_paths.rb @@ -10,7 +10,7 @@ module ActionView self._view_paths.freeze end - delegate :template_exists?, :view_paths, :formats, :formats=, + delegate :template_exists?, :any_templates?, :view_paths, :formats, :formats=, :locale, :locale=, :to => :lookup_context module ClassMethods diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index 9ff5f7126b..d4c5048bde 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 @@ -145,6 +146,11 @@ class TemplateDigestorTest < ActionView::TestCase end end + def test_nested_template_deps + nested_deps = ["messages/header", {"comments/comments"=>["comments/comment"]}, "messages/actions/move", "events/event", "messages/something_missing", "messages/something_missing_1", "messages/message", "messages/form"] + assert_equal nested_deps, nested_dependencies("messages/show") + end + def test_recursion_in_renders assert digest("level/recursion") # assert recursion is possible assert_not_nil digest("level/recursion") # assert digest is stored @@ -312,16 +318,17 @@ class TemplateDigestorTest < ActionView::TestCase options = options.dup finder.variants = options.delete(:variants) || [] - - ActionView::Digestor.digest({ name: template_name, finder: finder }.merge(options)) + ActionView::Digestor.digest(name: template_name, finder: finder, dependencies: (options[:dependencies] || [])) end def dependencies(template_name) - ActionView::Digestor.new(template_name, finder).dependencies + tree = ActionView::Digestor.tree(template_name, finder) + tree.children.map(&:name) end def nested_dependencies(template_name) - ActionView::Digestor.new(template_name, finder).nested_dependencies + tree = ActionView::Digestor.tree(template_name, finder) + tree.children.map(&:to_dep_map) end def finder diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index 229ef03879..913164fbc5 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,3 +1,5 @@ +## Rails 5.0.0.beta3 (February 24, 2016) ## + * Change the default adapter from inline to async. It's a better default as tests will then not mistakenly come to rely on behavior happening synchronously. This is especially important with things like jobs kicked off in Active Record lifecycle callbacks. diff --git a/activejob/lib/active_job/gem_version.rb b/activejob/lib/active_job/gem_version.rb index bc88221027..be4fabf545 100644 --- a/activejob/lib/active_job/gem_version.rb +++ b/activejob/lib/active_job/gem_version.rb @@ -8,7 +8,7 @@ module ActiveJob MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index ed0c05c1e5..e06b98736d 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -4,325 +4,321 @@ require 'active_support/core_ext/hash/keys' module ActiveJob # Provides helper methods for testing Active Job module TestHelper - extend ActiveSupport::Concern + delegate :enqueued_jobs, :enqueued_jobs=, + :performed_jobs, :performed_jobs=, + to: :queue_adapter - included do - def before_setup # :nodoc: - test_adapter = ActiveJob::QueueAdapters::TestAdapter.new + def before_setup # :nodoc: + test_adapter = ActiveJob::QueueAdapters::TestAdapter.new - @old_queue_adapters = (ActiveJob::Base.subclasses << ActiveJob::Base).select do |klass| - # only override explicitly set adapters, a quirk of `class_attribute` - klass.singleton_class.public_instance_methods(false).include?(:_queue_adapter) - end.map do |klass| - [klass, klass.queue_adapter].tap do - klass.queue_adapter = test_adapter - end + @old_queue_adapters = (ActiveJob::Base.subclasses << ActiveJob::Base).select do |klass| + # only override explicitly set adapters, a quirk of `class_attribute` + klass.singleton_class.public_instance_methods(false).include?(:_queue_adapter) + end.map do |klass| + [klass, klass.queue_adapter].tap do + klass.queue_adapter = test_adapter end - - clear_enqueued_jobs - clear_performed_jobs - super end - def after_teardown # :nodoc: - super - @old_queue_adapters.each do |(klass, adapter)| - klass.queue_adapter = adapter - end - end + clear_enqueued_jobs + clear_performed_jobs + super + end - # Asserts that the number of enqueued jobs matches the given number. - # - # def test_jobs - # assert_enqueued_jobs 0 - # HelloJob.perform_later('david') - # assert_enqueued_jobs 1 - # HelloJob.perform_later('abdelkader') - # assert_enqueued_jobs 2 - # end - # - # If a block is passed, that block should cause the specified number of - # jobs to be enqueued. - # - # def test_jobs_again - # assert_enqueued_jobs 1 do - # HelloJob.perform_later('cristian') - # end - # - # assert_enqueued_jobs 2 do - # HelloJob.perform_later('aaron') - # HelloJob.perform_later('rafael') - # end - # end - # - # The number of times a specific job is enqueued can be asserted. - # - # def test_logging_job - # assert_enqueued_jobs 1, only: LoggingJob do - # LoggingJob.perform_later - # HelloJob.perform_later('jeremy') - # end - # end - def assert_enqueued_jobs(number, only: nil) - if block_given? - original_count = enqueued_jobs_size(only: only) - yield - new_count = enqueued_jobs_size(only: only) - assert_equal number, new_count - original_count, "#{number} jobs expected, but #{new_count - original_count} were enqueued" - else - actual_count = enqueued_jobs_size(only: only) - assert_equal number, actual_count, "#{number} jobs expected, but #{actual_count} were enqueued" - end + def after_teardown # :nodoc: + super + @old_queue_adapters.each do |(klass, adapter)| + klass.queue_adapter = adapter end + end - # Asserts that no jobs have been enqueued. - # - # def test_jobs - # assert_no_enqueued_jobs - # HelloJob.perform_later('jeremy') - # assert_enqueued_jobs 1 - # end - # - # If a block is passed, that block should not cause any job to be enqueued. - # - # def test_jobs_again - # assert_no_enqueued_jobs do - # # No job should be enqueued from this block - # end - # end - # - # It can be asserted that no jobs of a specific kind are enqueued: - # - # def test_no_logging - # assert_no_enqueued_jobs only: LoggingJob do - # HelloJob.perform_later('jeremy') - # end - # end - # - # Note: This assertion is simply a shortcut for: - # - # assert_enqueued_jobs 0, &block - def assert_no_enqueued_jobs(only: nil, &block) - assert_enqueued_jobs 0, only: only, &block + # Asserts that the number of enqueued jobs matches the given number. + # + # def test_jobs + # assert_enqueued_jobs 0 + # HelloJob.perform_later('david') + # assert_enqueued_jobs 1 + # HelloJob.perform_later('abdelkader') + # assert_enqueued_jobs 2 + # end + # + # If a block is passed, that block should cause the specified number of + # jobs to be enqueued. + # + # def test_jobs_again + # assert_enqueued_jobs 1 do + # HelloJob.perform_later('cristian') + # end + # + # assert_enqueued_jobs 2 do + # HelloJob.perform_later('aaron') + # HelloJob.perform_later('rafael') + # end + # end + # + # The number of times a specific job is enqueued can be asserted. + # + # def test_logging_job + # assert_enqueued_jobs 1, only: LoggingJob do + # LoggingJob.perform_later + # HelloJob.perform_later('jeremy') + # end + # end + def assert_enqueued_jobs(number, only: nil) + if block_given? + original_count = enqueued_jobs_size(only: only) + yield + new_count = enqueued_jobs_size(only: only) + assert_equal number, new_count - original_count, "#{number} jobs expected, but #{new_count - original_count} were enqueued" + else + actual_count = enqueued_jobs_size(only: only) + assert_equal number, actual_count, "#{number} jobs expected, but #{actual_count} were enqueued" end + end - # Asserts that the number of performed jobs matches the given number. - # If no block is passed, <tt>perform_enqueued_jobs</tt> - # must be called around the job call. - # - # def test_jobs - # assert_performed_jobs 0 - # - # perform_enqueued_jobs do - # HelloJob.perform_later('xavier') - # end - # assert_performed_jobs 1 - # - # perform_enqueued_jobs do - # HelloJob.perform_later('yves') - # assert_performed_jobs 2 - # end - # end - # - # If a block is passed, that block should cause the specified number of - # jobs to be performed. - # - # def test_jobs_again - # assert_performed_jobs 1 do - # HelloJob.perform_later('robin') - # end - # - # assert_performed_jobs 2 do - # HelloJob.perform_later('carlos') - # HelloJob.perform_later('sean') - # end - # end - # - # The block form supports filtering. If the :only option is specified, - # then only the listed job(s) will be performed. - # - # def test_hello_job - # assert_performed_jobs 1, only: HelloJob do - # HelloJob.perform_later('jeremy') - # LoggingJob.perform_later - # end - # end - # - # An array may also be specified, to support testing multiple jobs. - # - # def test_hello_and_logging_jobs - # assert_nothing_raised do - # assert_performed_jobs 2, only: [HelloJob, LoggingJob] do - # HelloJob.perform_later('jeremy') - # LoggingJob.perform_later('stewie') - # RescueJob.perform_later('david') - # end - # end - # end - def assert_performed_jobs(number, only: nil) - if block_given? - original_count = performed_jobs.size - perform_enqueued_jobs(only: only) { yield } - new_count = performed_jobs.size - assert_equal number, new_count - original_count, - "#{number} jobs expected, but #{new_count - original_count} were performed" - else - performed_jobs_size = performed_jobs.size - assert_equal number, performed_jobs_size, "#{number} jobs expected, but #{performed_jobs_size} were performed" - end - end + # Asserts that no jobs have been enqueued. + # + # def test_jobs + # assert_no_enqueued_jobs + # HelloJob.perform_later('jeremy') + # assert_enqueued_jobs 1 + # end + # + # If a block is passed, that block should not cause any job to be enqueued. + # + # def test_jobs_again + # assert_no_enqueued_jobs do + # # No job should be enqueued from this block + # end + # end + # + # It can be asserted that no jobs of a specific kind are enqueued: + # + # def test_no_logging + # assert_no_enqueued_jobs only: LoggingJob do + # HelloJob.perform_later('jeremy') + # end + # end + # + # Note: This assertion is simply a shortcut for: + # + # assert_enqueued_jobs 0, &block + def assert_no_enqueued_jobs(only: nil, &block) + assert_enqueued_jobs 0, only: only, &block + end - # Asserts that no jobs have been performed. - # - # def test_jobs - # assert_no_performed_jobs - # - # perform_enqueued_jobs do - # HelloJob.perform_later('matthew') - # assert_performed_jobs 1 - # end - # end - # - # If a block is passed, that block should not cause any job to be performed. - # - # def test_jobs_again - # assert_no_performed_jobs do - # # No job should be performed from this block - # end - # end - # - # The block form supports filtering. If the :only option is specified, - # then only the listed job(s) will be performed. - # - # def test_hello_job - # assert_performed_jobs 1, only: HelloJob do - # HelloJob.perform_later('jeremy') - # LoggingJob.perform_later - # end - # end - # - # An array may also be specified, to support testing multiple jobs. - # - # def test_hello_and_logging_jobs - # assert_nothing_raised do - # assert_performed_jobs 2, only: [HelloJob, LoggingJob] do - # HelloJob.perform_later('jeremy') - # LoggingJob.perform_later('stewie') - # RescueJob.perform_later('david') - # end - # end - # end - # - # Note: This assertion is simply a shortcut for: - # - # assert_performed_jobs 0, &block - def assert_no_performed_jobs(only: nil, &block) - assert_performed_jobs 0, only: only, &block + # Asserts that the number of performed jobs matches the given number. + # If no block is passed, <tt>perform_enqueued_jobs</tt> + # must be called around the job call. + # + # def test_jobs + # assert_performed_jobs 0 + # + # perform_enqueued_jobs do + # HelloJob.perform_later('xavier') + # end + # assert_performed_jobs 1 + # + # perform_enqueued_jobs do + # HelloJob.perform_later('yves') + # assert_performed_jobs 2 + # end + # end + # + # If a block is passed, that block should cause the specified number of + # jobs to be performed. + # + # def test_jobs_again + # assert_performed_jobs 1 do + # HelloJob.perform_later('robin') + # end + # + # assert_performed_jobs 2 do + # HelloJob.perform_later('carlos') + # HelloJob.perform_later('sean') + # end + # end + # + # The block form supports filtering. If the :only option is specified, + # then only the listed job(s) will be performed. + # + # def test_hello_job + # assert_performed_jobs 1, only: HelloJob do + # HelloJob.perform_later('jeremy') + # LoggingJob.perform_later + # end + # end + # + # An array may also be specified, to support testing multiple jobs. + # + # def test_hello_and_logging_jobs + # assert_nothing_raised do + # assert_performed_jobs 2, only: [HelloJob, LoggingJob] do + # HelloJob.perform_later('jeremy') + # LoggingJob.perform_later('stewie') + # RescueJob.perform_later('david') + # end + # end + # end + def assert_performed_jobs(number, only: nil) + if block_given? + original_count = performed_jobs.size + perform_enqueued_jobs(only: only) { yield } + new_count = performed_jobs.size + assert_equal number, new_count - original_count, + "#{number} jobs expected, but #{new_count - original_count} were performed" + else + performed_jobs_size = performed_jobs.size + assert_equal number, performed_jobs_size, "#{number} jobs expected, but #{performed_jobs_size} were performed" end + end - # Asserts that the job passed in the block has been enqueued with the given arguments. - # - # def test_assert_enqueued_with - # assert_enqueued_with(job: MyJob, args: [1,2,3], queue: 'low') do - # MyJob.perform_later(1,2,3) - # end - # - # assert_enqueued_with(job: MyJob, at: Date.tomorrow.noon) do - # MyJob.set(wait_until: Date.tomorrow.noon).perform_later - # end - # end - def assert_enqueued_with(args = {}) - original_enqueued_jobs_count = enqueued_jobs.count - args.assert_valid_keys(:job, :args, :at, :queue) - serialized_args = serialize_args_for_assertion(args) - yield - in_block_jobs = enqueued_jobs.drop(original_enqueued_jobs_count) - matching_job = in_block_jobs.find do |job| - serialized_args.all? { |key, value| value == job[key] } - end - assert matching_job, "No enqueued job found with #{args}" - instantiate_job(matching_job) - end + # Asserts that no jobs have been performed. + # + # def test_jobs + # assert_no_performed_jobs + # + # perform_enqueued_jobs do + # HelloJob.perform_later('matthew') + # assert_performed_jobs 1 + # end + # end + # + # If a block is passed, that block should not cause any job to be performed. + # + # def test_jobs_again + # assert_no_performed_jobs do + # # No job should be performed from this block + # end + # end + # + # The block form supports filtering. If the :only option is specified, + # then only the listed job(s) will be performed. + # + # def test_hello_job + # assert_performed_jobs 1, only: HelloJob do + # HelloJob.perform_later('jeremy') + # LoggingJob.perform_later + # end + # end + # + # An array may also be specified, to support testing multiple jobs. + # + # def test_hello_and_logging_jobs + # assert_nothing_raised do + # assert_performed_jobs 2, only: [HelloJob, LoggingJob] do + # HelloJob.perform_later('jeremy') + # LoggingJob.perform_later('stewie') + # RescueJob.perform_later('david') + # end + # end + # end + # + # Note: This assertion is simply a shortcut for: + # + # assert_performed_jobs 0, &block + def assert_no_performed_jobs(only: nil, &block) + assert_performed_jobs 0, only: only, &block + end - # Asserts that the job passed in the block has been performed with the given arguments. - # - # def test_assert_performed_with - # assert_performed_with(job: MyJob, args: [1,2,3], queue: 'high') do - # MyJob.perform_later(1,2,3) - # end - # - # assert_performed_with(job: MyJob, at: Date.tomorrow.noon) do - # MyJob.set(wait_until: Date.tomorrow.noon).perform_later - # end - # end - def assert_performed_with(args = {}) - original_performed_jobs_count = performed_jobs.count - args.assert_valid_keys(:job, :args, :at, :queue) - serialized_args = serialize_args_for_assertion(args) - perform_enqueued_jobs { yield } - in_block_jobs = performed_jobs.drop(original_performed_jobs_count) - matching_job = in_block_jobs.find do |job| - serialized_args.all? { |key, value| value == job[key] } - end - assert matching_job, "No performed job found with #{args}" - instantiate_job(matching_job) + # Asserts that the job passed in the block has been enqueued with the given arguments. + # + # def test_assert_enqueued_with + # assert_enqueued_with(job: MyJob, args: [1,2,3], queue: 'low') do + # MyJob.perform_later(1,2,3) + # end + # + # assert_enqueued_with(job: MyJob, at: Date.tomorrow.noon) do + # MyJob.set(wait_until: Date.tomorrow.noon).perform_later + # end + # end + def assert_enqueued_with(args = {}) + original_enqueued_jobs_count = enqueued_jobs.count + args.assert_valid_keys(:job, :args, :at, :queue) + serialized_args = serialize_args_for_assertion(args) + yield + in_block_jobs = enqueued_jobs.drop(original_enqueued_jobs_count) + matching_job = in_block_jobs.find do |job| + serialized_args.all? { |key, value| value == job[key] } end + assert matching_job, "No enqueued job found with #{args}" + instantiate_job(matching_job) + end - def perform_enqueued_jobs(only: nil) - old_perform_enqueued_jobs = queue_adapter.perform_enqueued_jobs - old_perform_enqueued_at_jobs = queue_adapter.perform_enqueued_at_jobs - old_filter = queue_adapter.filter - - begin - queue_adapter.perform_enqueued_jobs = true - queue_adapter.perform_enqueued_at_jobs = true - queue_adapter.filter = only - yield - ensure - queue_adapter.perform_enqueued_jobs = old_perform_enqueued_jobs - queue_adapter.perform_enqueued_at_jobs = old_perform_enqueued_at_jobs - queue_adapter.filter = old_filter - end + # Asserts that the job passed in the block has been performed with the given arguments. + # + # def test_assert_performed_with + # assert_performed_with(job: MyJob, args: [1,2,3], queue: 'high') do + # MyJob.perform_later(1,2,3) + # end + # + # assert_performed_with(job: MyJob, at: Date.tomorrow.noon) do + # MyJob.set(wait_until: Date.tomorrow.noon).perform_later + # end + # end + def assert_performed_with(args = {}) + original_performed_jobs_count = performed_jobs.count + args.assert_valid_keys(:job, :args, :at, :queue) + serialized_args = serialize_args_for_assertion(args) + perform_enqueued_jobs { yield } + in_block_jobs = performed_jobs.drop(original_performed_jobs_count) + matching_job = in_block_jobs.find do |job| + serialized_args.all? { |key, value| value == job[key] } end + assert matching_job, "No performed job found with #{args}" + instantiate_job(matching_job) + end + + def perform_enqueued_jobs(only: nil) + old_perform_enqueued_jobs = queue_adapter.perform_enqueued_jobs + old_perform_enqueued_at_jobs = queue_adapter.perform_enqueued_at_jobs + old_filter = queue_adapter.filter - def queue_adapter - ActiveJob::Base.queue_adapter + begin + queue_adapter.perform_enqueued_jobs = true + queue_adapter.perform_enqueued_at_jobs = true + queue_adapter.filter = only + yield + ensure + queue_adapter.perform_enqueued_jobs = old_perform_enqueued_jobs + queue_adapter.perform_enqueued_at_jobs = old_perform_enqueued_at_jobs + queue_adapter.filter = old_filter end + end - delegate :enqueued_jobs, :enqueued_jobs=, - :performed_jobs, :performed_jobs=, - to: :queue_adapter + def queue_adapter + ActiveJob::Base.queue_adapter + end - private - def clear_enqueued_jobs # :nodoc: - enqueued_jobs.clear - end + private + def clear_enqueued_jobs # :nodoc: + enqueued_jobs.clear + end - def clear_performed_jobs # :nodoc: - performed_jobs.clear - end + def clear_performed_jobs # :nodoc: + performed_jobs.clear + end - def enqueued_jobs_size(only: nil) # :nodoc: - if only - enqueued_jobs.count { |job| Array(only).include?(job.fetch(:job)) } - else - enqueued_jobs.count - end + def enqueued_jobs_size(only: nil) # :nodoc: + if only + enqueued_jobs.count { |job| Array(only).include?(job.fetch(:job)) } + else + enqueued_jobs.count end + end - def serialize_args_for_assertion(args) # :nodoc: - args.dup.tap do |serialized_args| - serialized_args[:args] = ActiveJob::Arguments.serialize(serialized_args[:args]) if serialized_args[:args] - serialized_args[:at] = serialized_args[:at].to_f if serialized_args[:at] - end + def serialize_args_for_assertion(args) # :nodoc: + args.dup.tap do |serialized_args| + serialized_args[:args] = ActiveJob::Arguments.serialize(serialized_args[:args]) if serialized_args[:args] + serialized_args[:at] = serialized_args[:at].to_f if serialized_args[:at] end + end - def instantiate_job(payload) # :nodoc: - job = payload[:job].new(*payload[:args]) - job.scheduled_at = Time.at(payload[:at]) if payload.key?(:at) - job.queue_name = payload[:queue] - job - end - end + def instantiate_job(payload) # :nodoc: + job = payload[:job].new(*payload[:args]) + job.scheduled_at = Time.at(payload[:at]) if payload.key?(:at) + job.queue_name = payload[:queue] + job + end end end diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 318e507ff1..fb7ab5cb40 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,8 @@ +## Rails 5.0.0.beta3 (February 24, 2016) ## + +* No changes. + + ## Rails 5.0.0.beta2 (February 01, 2016) ## * No changes. diff --git a/activemodel/lib/active_model/gem_version.rb b/activemodel/lib/active_model/gem_version.rb index 94514a0657..62862aa4e9 100644 --- a/activemodel/lib/active_model/gem_version.rb +++ b/activemodel/lib/active_model/gem_version.rb @@ -8,7 +8,7 @@ module ActiveModel MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") 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 c18403865f..39fe217777 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Ensure that the Suppressor runs before validations. + + This moves the suppressor up to be run before validations rather than after + validations. There's no reason to validate a record you aren't planning on saving. + + *Eileen M. Uchitelle* + +## Rails 5.0.0.beta3 (February 24, 2016) ## + * 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. @@ -1452,18 +1461,6 @@ *Chris Sinjakli* -* Validation errors would be raised for parent records when an association - was saved when the parent had `validate: false`. It should not be the - responsibility of the model to validate an associated object unless the - object was created or modified by the parent. - - This fixes the issue by skipping validations if the parent record is - persisted, not changed, and not marked for destruction. - - Fixes #17621. - - *Eileen M. Uchitelle*, *Aaron Patterson* - * Fix n+1 query problem when eager loading nil associations (fixes #18312) *Sammy Larbi* diff --git a/activerecord/lib/active_record/gem_version.rb b/activerecord/lib/active_record/gem_version.rb index aa1f5c4fb4..73be4cb271 100644 --- a/activerecord/lib/active_record/gem_version.rb +++ b/activerecord/lib/active_record/gem_version.rb @@ -8,7 +8,7 @@ module ActiveRecord MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") 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/suppressor.rb b/activerecord/lib/active_record/suppressor.rb index b3644bf569..8ec4b48d31 100644 --- a/activerecord/lib/active_record/suppressor.rb +++ b/activerecord/lib/active_record/suppressor.rb @@ -37,7 +37,11 @@ module ActiveRecord end end - def create_or_update(*args) # :nodoc: + def save(*) # :nodoc: + SuppressorRegistry.suppressed[self.class.name] ? true : super + end + + def save!(*) # :nodoc: SuppressorRegistry.suppressed[self.class.name] ? true : super end end 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 88c272657f..4a80cda0b8 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -11,15 +11,14 @@ 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) + relation = relation.where.not(finder_class.primary_key => record.id_was || record.id) else raise UnknownPrimaryKey.new(finder_class, "Can not validate uniqueness for persisted record without primary key.") end 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..f191eff5bf 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,23 +39,34 @@ 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 && !application_record_exist? + template 'application_record.rb', application_record_file_name + 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 end - def determine_default_parent_class - application_record = nil + def application_record_exist? + file_exist = nil + in_root { file_exist = File.exist?(application_record_file_name) } + file_exist + end - in_root do - application_record = if mountable_engine? - File.exist?("app/models/#{namespaced_path}/application_record.rb") - else - File.exist?('app/models/application_record.rb') - end + def application_record_file_name + @application_record_file_name ||= if mountable_engine? + "app/models/#{namespaced_path}/application_record.rb" + else + 'app/models/application_record.rb' end + end - if application_record + def determine_default_parent_class + if application_record_exist? "ApplicationRecord" else "ActiveRecord::Base" 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..60050e0bf8 --- /dev/null +++ b/activerecord/lib/rails/generators/active_record/model/templates/application_record.rb @@ -0,0 +1,5 @@ +<% module_namespacing do -%> +class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true +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..6a6250eec3 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -516,13 +516,12 @@ class MigrationTest < ActiveRecord::TestCase data_column = columns.detect { |c| c.name == "data" } assert_nil data_column.default - + ensure Person.connection.drop_table :binary_testings, if_exists: true end unless mysql_enforcing_gtid_consistency? def test_create_table_with_query - Person.connection.drop_table :table_from_query_testings rescue nil Person.connection.create_table(:person, force: true) Person.connection.create_table :table_from_query_testings, as: "SELECT id FROM person" @@ -530,12 +529,11 @@ class MigrationTest < ActiveRecord::TestCase columns = Person.connection.columns(:table_from_query_testings) assert_equal 1, columns.length assert_equal "id", columns.first.name - + ensure Person.connection.drop_table :table_from_query_testings rescue nil end def test_create_table_with_query_from_relation - Person.connection.drop_table :table_from_query_testings rescue nil Person.connection.create_table(:person, force: true) Person.connection.create_table :table_from_query_testings, as: Person.select(:id) @@ -543,7 +541,7 @@ class MigrationTest < ActiveRecord::TestCase columns = Person.connection.columns(:table_from_query_testings) assert_equal 1, columns.length assert_equal "id", columns.first.name - + ensure Person.connection.drop_table :table_from_query_testings rescue nil end end @@ -586,8 +584,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 +592,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 @@ -726,7 +729,7 @@ class ReservedWordsMigrationTest < ActiveRecord::TestCase connection.add_index :values, :value connection.remove_index :values, :column => :value end - + ensure connection.drop_table :values rescue nil end end @@ -738,11 +741,11 @@ 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 - + ensure connection.drop_table :values rescue nil end 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/suppressor_test.rb b/activerecord/test/cases/suppressor_test.rb index 72c5c16555..7d44e36419 100644 --- a/activerecord/test/cases/suppressor_test.rb +++ b/activerecord/test/cases/suppressor_test.rb @@ -46,7 +46,18 @@ class SuppressorTest < ActiveRecord::TestCase Notification.suppress { UserWithNotification.create! } assert_difference -> { Notification.count } do - Notification.create! + Notification.create!(message: "New Comment") + end + end + + def test_suppresses_validations_on_create + assert_no_difference -> { Notification.count } do + Notification.suppress do + User.create + User.create! + User.new.save + User.new.save! + end end end end 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 4b6470393e..78263fd955 100644 --- a/activerecord/test/cases/validations/length_validation_test.rb +++ b/activerecord/test/cases/validations/length_validation_test.rb @@ -61,20 +61,6 @@ 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_length_of_virtual_attribute_on_model repair_validations(Pet) do Pet.send(:attr_accessor, :nickname) 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..4c14d93c66 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,18 @@ class BigIntReverseTest < ActiveRecord::Base validates :engines_count, uniqueness: true end +class CoolTopic < Topic + validates_uniqueness_of :id +end + +class TopicWithAfterCreate < Topic + after_create :set_author + + def set_author + update_attributes!(:author_name => "#{title} #{id}") + end +end + class UniquenessValidationTest < ActiveRecord::TestCase INT_MAX_VALUE = 2147483647 @@ -412,23 +425,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 +476,35 @@ class UniquenessValidationTest < ActiveRecord::TestCase assert t.valid?, "Should be valid" assert t.save, "Should still save t as unique" end + + def test_validate_uniqueness_with_after_create_performing_save + TopicWithAfterCreate.validates_uniqueness_of(:title) + topic = TopicWithAfterCreate.create!(:title => "Title1") + assert topic.author_name.start_with?("Title1") + + topic2 = TopicWithAfterCreate.new(:title => "Title1") + refute topic2.valid? + assert_equal(["has already been taken"], topic2.errors[:title]) + 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/notification.rb b/activerecord/test/models/notification.rb index b4b4b8f1b6..82edc64b68 100644 --- a/activerecord/test/models/notification.rb +++ b/activerecord/test/models/notification.rb @@ -1,2 +1,3 @@ class Notification < ActiveRecord::Base + validates_presence_of :message 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..1a169d36be 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,14 @@ +## Rails 5.0.0.beta3 (February 24, 2016) ## + +* 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 +17,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/deprecation/reporting.rb b/activesupport/lib/active_support/deprecation/reporting.rb index f89fc0fe14..35f084dd7a 100644 --- a/activesupport/lib/active_support/deprecation/reporting.rb +++ b/activesupport/lib/active_support/deprecation/reporting.rb @@ -1,3 +1,5 @@ +require 'rbconfig' + module ActiveSupport class Deprecation module Reporting @@ -81,17 +83,17 @@ module ActiveSupport def extract_callstack(callstack) return _extract_callstack(callstack) if callstack.first.is_a? String - rails_gem_root = File.expand_path("../../../../..", __FILE__) + "/" offending_line = callstack.find { |frame| - frame.absolute_path && !frame.absolute_path.start_with?(rails_gem_root) + frame.absolute_path && !ignored_callstack(frame.absolute_path) } || callstack.first + [offending_line.path, offending_line.lineno, offending_line.label] end def _extract_callstack(callstack) warn "Please pass `caller_locations` to the deprecation API" if $VERBOSE - rails_gem_root = File.expand_path("../../../../..", __FILE__) + "/" - offending_line = callstack.find { |line| !line.start_with?(rails_gem_root) } || callstack.first + offending_line = callstack.find { |line| !ignored_callstack(line) } || callstack.first + if offending_line if md = offending_line.match(/^(.+?):(\d+)(?::in `(.*?)')?/) md.captures @@ -100,6 +102,12 @@ module ActiveSupport end end end + + RAILS_GEM_ROOT = File.expand_path("../../../../..", __FILE__) + "/" + + def ignored_callstack(path) + path.start_with?(RAILS_GEM_ROOT) || path.start_with?(RbConfig::CONFIG['rubylibdir']) + end end end end diff --git a/activesupport/lib/active_support/gem_version.rb b/activesupport/lib/active_support/gem_version.rb index fc08273b6d..4166ffc2fb 100644 --- a/activesupport/lib/active_support/gem_version.rb +++ b/activesupport/lib/active_support/gem_version.rb @@ -8,7 +8,7 @@ module ActiveSupport MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/activesupport/lib/active_support/logger.rb b/activesupport/lib/active_support/logger.rb index 7626b28108..de48e717b6 100644 --- a/activesupport/lib/active_support/logger.rb +++ b/activesupport/lib/active_support/logger.rb @@ -1,8 +1,10 @@ require 'active_support/logger_silence' +require 'active_support/logger_thread_safe_level' require 'logger' module ActiveSupport class Logger < ::Logger + include ActiveSupport::LoggerThreadSafeLevel include LoggerSilence # Returns true if the logger destination matches one of the sources @@ -48,6 +50,11 @@ module ActiveSupport logger.level = level super(level) end + + define_method(:local_level=) do |level| + logger.local_level = level if logger.respond_to?(:local_level=) + super(level) if respond_to?(:local_level=) + end end end diff --git a/activesupport/lib/active_support/logger_silence.rb b/activesupport/lib/active_support/logger_silence.rb index 125d81d973..3eb8098c77 100644 --- a/activesupport/lib/active_support/logger_silence.rb +++ b/activesupport/lib/active_support/logger_silence.rb @@ -7,39 +7,22 @@ module LoggerSilence included do cattr_accessor :silencer - attr_reader :local_levels self.silencer = true end - def after_initialize - @local_levels = Concurrent::Map.new(:initial_capacity => 2) - end - - def local_log_id - Thread.current.__id__ - end - - def level - local_levels[local_log_id] || super - end - # Silences the logger for the duration of the block. def silence(temporary_level = Logger::ERROR) if silencer begin - old_local_level = local_levels[local_log_id] - local_levels[local_log_id] = temporary_level + old_local_level = local_level + self.local_level = temporary_level yield self ensure - if old_local_level - local_levels[local_log_id] = old_local_level - else - local_levels.delete(local_log_id) - end + self.local_level = old_local_level end else yield self end end -end
\ No newline at end of file +end diff --git a/activesupport/lib/active_support/logger_thread_safe_level.rb b/activesupport/lib/active_support/logger_thread_safe_level.rb new file mode 100644 index 0000000000..5fedb5e689 --- /dev/null +++ b/activesupport/lib/active_support/logger_thread_safe_level.rb @@ -0,0 +1,31 @@ +require 'active_support/concern' + +module ActiveSupport + module LoggerThreadSafeLevel # :nodoc: + extend ActiveSupport::Concern + + def after_initialize + @local_levels = Concurrent::Map.new(initial_capacity: 2) + end + + def local_log_id + Thread.current.__id__ + end + + def local_level + @local_levels[local_log_id] + end + + def local_level=(level) + if level + @local_levels[local_log_id] = level + else + @local_levels.delete(local_log_id) + end + end + + def level + local_level || super + end + end +end 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 39fa3cedbe..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 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/activesupport/test/logger_test.rb b/activesupport/test/logger_test.rb index 317e09b7f2..5a91420f1e 100644 --- a/activesupport/test/logger_test.rb +++ b/activesupport/test/logger_test.rb @@ -141,6 +141,50 @@ class LoggerTest < ActiveSupport::TestCase assert @output.string.include?("THIS IS HERE") end + def test_logger_silencing_works_for_broadcast + another_output = StringIO.new + another_logger = Logger.new(another_output) + + @logger.extend Logger.broadcast(another_logger) + + @logger.debug "CORRECT DEBUG" + @logger.silence do + @logger.debug "FAILURE" + @logger.error "CORRECT ERROR" + end + + assert @output.string.include?("CORRECT DEBUG") + assert @output.string.include?("CORRECT ERROR") + assert_not @output.string.include?("FAILURE") + + assert another_output.string.include?("CORRECT DEBUG") + assert another_output.string.include?("CORRECT ERROR") + assert_not another_output.string.include?("FAILURE") + end + + def test_broadcast_silencing_does_not_break_plain_ruby_logger + another_output = StringIO.new + another_logger = ::Logger.new(another_output) + + @logger.extend Logger.broadcast(another_logger) + + @logger.debug "CORRECT DEBUG" + @logger.silence do + @logger.debug "FAILURE" + @logger.error "CORRECT ERROR" + end + + assert @output.string.include?("CORRECT DEBUG") + assert @output.string.include?("CORRECT ERROR") + assert_not @output.string.include?("FAILURE") + + assert another_output.string.include?("CORRECT DEBUG") + assert another_output.string.include?("CORRECT ERROR") + assert another_output.string.include?("FAILURE") + # We can't silence plain ruby Logger cause with thread safety + # but at least we don't break it + end + def test_logger_level_per_object_thread_safety logger1 = Logger.new(StringIO.new) logger2 = Logger.new(StringIO.new) diff --git a/guides/CHANGELOG.md b/guides/CHANGELOG.md index d58016053b..d35d0f1976 100644 --- a/guides/CHANGELOG.md +++ b/guides/CHANGELOG.md @@ -1,3 +1,8 @@ +## Rails 5.0.0.beta3 (February 24, 2016) ## + +* No changes. + + ## Rails 5.0.0.beta2 (February 01, 2016) ## * No changes. diff --git a/guides/source/5_0_release_notes.md b/guides/source/5_0_release_notes.md index f5abbd0cd4..dc631e5cb9 100644 --- a/guides/source/5_0_release_notes.md +++ b/guides/source/5_0_release_notes.md @@ -256,6 +256,12 @@ Please refer to the [Changelog][action-pack] for detailed changes. * Rails will only generate "weak", instead of strong ETags. ([Pull Request](https://github.com/rails/rails/pull/17573)) +* Controller actions without an explicit `render` call and with no + corresponding templates will render `head :no_content` implicitly + instead of raising an error. + (Pull Request [1](https://github.com/rails/rails/pull/19377), + [2](https://github.com/rails/rails/pull/23827)) + Action View ------------- @@ -316,6 +322,9 @@ Please refer to the [Changelog][action-mailer] for detailed changes. * Template lookup now respects default locale and I18n fallbacks. ([commit](https://github.com/rails/rails/commit/ecb1981b)) +* Template can use fragment cache like Action View template. + ([Pull Request](https://github.com/rails/rails/pull/22825)) + * Added `_mailer` suffix to mailers created via generator, following the same naming convention used in controllers and jobs. ([Pull Request](https://github.com/rails/rails/pull/18074)) @@ -327,6 +336,9 @@ Please refer to the [Changelog][action-mailer] for detailed changes. the mailer queue name. ([Pull Request](https://github.com/rails/rails/pull/18587)) +* Added `config.action_mailer.perform_caching` configuration to determine whether your templates should perform caching or not. + ([Pull Request](https://github.com/rails/rails/pull/22825)) + Active Record ------------- diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md index 91ea4efb55..558c16f5b0 100644 --- a/guides/source/action_mailer_basics.md +++ b/guides/source/action_mailer_basics.md @@ -407,6 +407,22 @@ use the rendered text for the text part. The render command is the same one used inside of Action Controller, so you can use all the same options, such as `:text`, `:inline` etc. +#### Caching mailer view + +You can do cache in mailer views like in application views using `cache` method. + +``` +<% cache do %> + <%= @company.name %> +<% end %> +``` + +And in order to use this feature, you need to configure your application with this: + +``` + config.action_mailer.perform_caching = true +``` + ### Action Mailer Layouts Just like controller views, you can also have mailer layouts. The layout name diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md index 5e6eae1071..29e0943741 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -599,7 +599,7 @@ This would add something like "Process data files (0.34523)" to the log, which y #### cache -A method for caching fragments of a view rather than an entire action or page. This technique is useful for caching pieces like menus, lists of news topics, static HTML fragments, and so on. This method takes a block that contains the content you wish to cache. See `ActionController::Caching::Fragments` for more information. +A method for caching fragments of a view rather than an entire action or page. This technique is useful for caching pieces like menus, lists of news topics, static HTML fragments, and so on. This method takes a block that contains the content you wish to cache. See `AbstractController::Caching::Fragments` for more information. ```erb <% cache do %> diff --git a/guides/source/api_app.md b/guides/source/api_app.md index 0598b9c7fa..8dba914923 100644 --- a/guides/source/api_app.md +++ b/guides/source/api_app.md @@ -166,6 +166,23 @@ class definition: config.api_only = true ``` +In `config/environments/development.rb`, set `config.debug_exception_response_format` +to configure the format used in responses when errors occur in development mode. + +To render an HTML page with debugging information, use the value `:default`. + +```ruby +config.debug_exception_response_format = :default +``` + +To render debugging information preserving the response format, use the value `:api`. + +```ruby +config.debug_exception_response_format = :api +``` + +By default, `config.debug_exception_response_format` is set to `:api`. + Finally, inside `app/controllers/application_controller.rb`, instead of: ```ruby 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/caching_with_rails.md b/guides/source/caching_with_rails.md index 3a1a1ccfe6..f26019c72e 100644 --- a/guides/source/caching_with_rails.md +++ b/guides/source/caching_with_rails.md @@ -521,6 +521,14 @@ class ProductsController < ApplicationController end ``` +### A note on weak ETags + +Etags generated by Rails are weak by default. Weak etags allow symantically equivalent responses to have the same etags, even if their bodies do not match exactly. This is useful when we don't want the page to be regenerated for minor changes in response body. If you absolutely need to generate a strong etag, it can be assigned to the header directly. + +```ruby + response.add_header "ETag", Digest::MD5.hexdigest(response.body) +``` + References ---------- diff --git a/guides/source/configuring.md b/guides/source/configuring.md index a5fb396f15..b83d25c683 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -98,13 +98,15 @@ application. Accepts a valid week day symbol (e.g. `:monday`). * `config.exceptions_app` sets the exceptions application invoked by the ShowException middleware when an exception happens. Defaults to `ActionDispatch::PublicExceptions.new(Rails.public_path)`. +* `config.debug_exception_response_format` sets the format used in responses when errors occur in development mode. + * `config.file_watcher` is the class used to detect file updates in the file system when `config.reload_classes_only_on_change` is true. Rails ships with `ActiveSupport::FileUpdateChecker`, the default, and `ActiveSupport::EventedFileUpdateChecker` (this one depends on the [listen](https://github.com/guard/listen) gem). Custom classes must conform to the `ActiveSupport::FileUpdateChecker` API. * `config.filter_parameters` used for filtering out the parameters that you don't want shown in the logs, such as passwords or credit card numbers. New applications filter out passwords by adding the following `config.filter_parameters+=[:password]` in `config/initializers/filter_parameter_logging.rb`. -* `config.force_ssl` forces all requests to be served over HTTPS by using the `ActionDispatch::SSL` middleware. This can be configured by setting `config.ssl_options` - see the [ActionDispatch::SSL documentation](http://edgeapi.rubyonrails.org/classes/ActionDispatch/SSL.html) for details. +* `config.force_ssl` forces all requests to be served over HTTPS by using the `ActionDispatch::SSL` middleware, and sets `config.action_mailer.default_url_options` to be `{ protocol: 'https' }`. This can be configured by setting `config.ssl_options` - see the [ActionDispatch::SSL documentation](http://edgeapi.rubyonrails.org/classes/ActionDispatch/SSL.html) for details. * `config.log_formatter` defines the formatter of the Rails logger. This option defaults to an instance of `ActiveSupport::Logger::SimpleFormatter` for all modes except production, where it defaults to `Logger::Formatter`. @@ -531,6 +533,9 @@ There are a number of settings available on `config.action_mailer`: * `config.action_mailer.deliver_later_queue_name` specifies the queue name for mailers. By default this is `mailers`. +* `config.action_mailer.perform_caching` specifies whether the mailer templates should perform fragment caching or not. By default this is false in all environments. + + ### Configuring Active Support There are a few configuration options available in Active Support: @@ -610,6 +615,17 @@ There are a few configuration options available in Active Support: * `config.active_job.logger` accepts a logger conforming to the interface of Log4r or the default Ruby Logger class, which is then used to log information from Active Job. You can retrieve this logger by calling `logger` on either an Active Job class or an Active Job instance. Set to `nil` to disable logging. +### Configuring Action Cable + +* `config.action_cable.url` accepts a string for the URL for where + you are hosting your Action Cable server. You would use this option +if you are running Action Cable servers that are separated from your +main application. +* `config.action_cable.mount_path` accepts a string for where to mount Action + Cable, as apart of the main server process. Defaults to `/cable`. +You can set this as nil to not mount Action Cable as apart of your +normal Rails server. + ### Configuring a Database Just about every Rails application will interact with a database. You can connect to the database by setting an environment variable `ENV['DATABASE_URL']` or by using a configuration file called `config/database.yml`. diff --git a/guides/source/debugging_rails_applications.md b/guides/source/debugging_rails_applications.md index 35ad6eb705..713d95a3c1 100644 --- a/guides/source/debugging_rails_applications.md +++ b/guides/source/debugging_rails_applications.md @@ -109,18 +109,18 @@ It can also be useful to save information to log files at runtime. Rails maintai Rails makes use of the `ActiveSupport::Logger` class to write log information. Other loggers, such as `Log4r`, may also be substituted. -You can specify an alternative logger in `environment.rb` or any other environment file, for example: +You can specify an alternative logger in `config/application.rb`, for example: ```ruby -Rails.logger = Logger.new(STDOUT) -Rails.logger = Log4r::Logger.new("Application Log") +config.logger = Logger.new(STDOUT) +config.logger = Log4r::Logger.new("Application Log") ``` Or in the `Initializer` section, add _any_ of the following ```ruby -config.logger = Logger.new(STDOUT) -config.logger = Log4r::Logger.new("Application Log") +Rails.logger = Logger.new(STDOUT) +Rails.logger = Log4r::Logger.new("Application Log") ``` TIP: By default, each log is created under `Rails.root/log/` and the log file is named after the environment in which the application is running. 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/getting_started.md b/guides/source/getting_started.md index 8eb3b6190f..4431512eda 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -298,26 +298,30 @@ Open the file `config/routes.rb` in your editor. Rails.application.routes.draw do get 'welcome/index' - # The priority is based upon order of creation: - # first created -> highest priority. - # See how all your routes lay out with "bin/rails routes". - # - # You can have the root of your site routed with "root" - # root 'welcome#index' - # - # ... + # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html + + # Serve websocket cable requests in-process + # mount ActionCable.server => '/cable' +end ``` This is your application's _routing file_ which holds entries in a special [DSL (domain-specific language)](http://en.wikipedia.org/wiki/Domain-specific_language) that tells Rails how to connect incoming requests to -controllers and actions. This file contains many sample routes on commented -lines, and one of them actually shows you how to connect the root of your site -to a specific controller and action. Find the line beginning with `root` and -uncomment it. It should look something like the following: +controllers and actions. +Edit this file by adding the line of code `root 'welcome#index'`. +It should look something like the following: ```ruby -root 'welcome#index' +Rails.application.routes.draw do + get 'welcome/index' + + # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html + + # Serve websocket cable requests in-process + # mount ActionCable.server => '/cable' + root 'welcome#index' +end ``` `root 'welcome#index'` tells Rails to map requests to the root of the @@ -348,7 +352,7 @@ operations are referred to as _CRUD_ operations. Rails provides a `resources` method which can be used to declare a standard REST resource. You need to add the _article resource_ to the -`config/routes.rb` as follows: +`config/routes.rb` so the file will look as follows: ```ruby Rails.application.routes.draw do @@ -625,7 +629,7 @@ end The `render` method here is taking a very simple hash with a key of `:plain` and value of `params[:article].inspect`. The `params` method is the object which represents the parameters (or fields) coming in from the form. The `params` -method returns an `ActiveSupport::HashWithIndifferentAccess` object, which +method returns an `ActionController::Parameters` object, which allows you to access the keys of the hash using either strings or symbols. In this situation, the only parameters that matter are the ones from the form. @@ -635,7 +639,7 @@ If you re-submit the form one more time you'll now no longer get the missing template error. Instead, you'll see something that looks like the following: ```ruby -{"title"=>"First article!", "text"=>"This is my first article."} +<ActionController::Parameters {"title"=>"First Article!", "text"=>"This is my first article."} permitted: false> ``` This action is now displaying the parameters for the article that are coming in diff --git a/guides/source/rails_application_templates.md b/guides/source/rails_application_templates.md index 5a46baff2d..3bb5d3c8a6 100644 --- a/guides/source/rails_application_templates.md +++ b/guides/source/rails_application_templates.md @@ -22,7 +22,7 @@ $ rails new blog -m ~/template.rb $ rails new blog -m http://example.com/template.rb ``` -You can use the rake task `rails:template` to apply templates to an existing Rails application. The location of the template needs to be passed in to an environment variable named LOCATION. Again, this can either be path to a file or a URL. +You can use the task `rails:template` to apply templates to an existing Rails application. The location of the template needs to be passed in to an environment variable named LOCATION. Again, this can either be path to a file or a URL. ```bash $ bin/rails rails:template LOCATION=~/template.rb @@ -38,7 +38,7 @@ The Rails templates API is easy to understand. Here's an example of a typical Ra # template.rb generate(:scaffold, "person name:string") route "root to: 'people#index'" -rake("db:migrate") +rails_command("db:migrate") after_bundle do git :init @@ -175,18 +175,24 @@ Executes an arbitrary command. Just like the backticks. Let's say you want to re run "rm README.rdoc" ``` -### rake(command, options = {}) +### rails_command(command, options = {}) -Runs the supplied rake tasks in the Rails application. Let's say you want to migrate the database: +Runs the supplied task in the Rails application. Let's say you want to migrate the database: ```ruby -rake "db:migrate" +rails_command "db:migrate" ``` -You can also run rake tasks with a different Rails environment: +You can also run tasks with a different Rails environment: ```ruby -rake "db:migrate", env: 'production' +rails_command "db:migrate", env: 'production' +``` + +You can also run tasks as a super-user: + +```ruby +rails_command "log:clear", sudo: true ``` ### route(routing_code) @@ -226,7 +232,7 @@ CODE These methods let you ask questions from templates and decide the flow based on the user's answer. Let's say you want to Freeze Rails only if the user wants to: ```ruby -rake("rails:freeze:gems") if yes?("Freeze rails gems?") +rails_command("rails:freeze:gems") if yes?("Freeze rails gems?") # no?(question) acts just the opposite. ``` 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/CHANGELOG.md b/railties/CHANGELOG.md index a3be5356a1..5a4ad2ff99 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,19 @@ +* Enable HSTS with IncludeSudomains header for new applications. + + *Egor Homakov*, *Prathamesh Sonpatki* + +## Rails 5.0.0.beta3 (February 24, 2016) ## + +* Alias `rake` with `rails_command` in the Rails Application Templates API + following Rails 5 convention of preferring "rails" to "rake" to run tasks. + + *claudiob* + +* Generate applications with an option to log to STDOUT in production + using the environment variable `RAILS_LOG_TO_STDOUT`. + + *Richard Schneeman* + * Change fail fast of `bin/rails test` interrupts run on error. *Yuji Yaginuma* 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/gem_version.rb b/railties/lib/rails/gem_version.rb index 93e0151602..081222425c 100644 --- a/railties/lib/rails/gem_version.rb +++ b/railties/lib/rails/gem_version.rb @@ -8,7 +8,7 @@ module Rails MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 9ca731347a..5fa487b78e 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -216,8 +216,9 @@ module Rails log :rake, command env = options[:env] || ENV["RAILS_ENV"] || 'development' sudo = options[:sudo] && RbConfig::CONFIG['host_os'] !~ /mswin|mingw/ ? 'sudo ' : '' - in_root { run("#{sudo}#{extify(:rake)} #{command} RAILS_ENV=#{env}", verbose: false) } + in_root { run("#{sudo}#{extify(:rails)} #{command} RAILS_ENV=#{env}", verbose: false) } end + alias :rails_command :rake # Just run the capify command in root # diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 9adfcc6ee7..89341e6fa2 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -181,7 +181,7 @@ module Rails def webserver_gemfile_entry return [] if options[:skip_puma] comment = 'Use Puma as the app server' - GemfileEntry.new('puma', nil, comment) + GemfileEntry.new('puma', '~> 3.0', comment) end def include_all_railties? @@ -311,12 +311,7 @@ module Rails end def coffee_gemfile_entry - comment = 'Use CoffeeScript for .coffee assets and views' - if options.dev? || options.edge? - GemfileEntry.github 'coffee-rails', 'rails/coffee-rails', nil, comment - else - GemfileEntry.version 'coffee-rails', '~> 4.1.0', comment - end + GemfileEntry.version 'coffee-rails', '~> 4.1.0', 'Use CoffeeScript for .coffee assets and views' end def javascript_gemfile_entry @@ -328,8 +323,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..4234706452 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -91,6 +91,8 @@ 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') + ssl_options_exist = File.exist?('config/initializers/ssl_options.rb') config @@ -105,6 +107,14 @@ 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 + + unless ssl_options_exist + remove_file 'config/initializers/ssl_options.rb' + end end def database_yml diff --git a/railties/lib/rails/generators/rails/app/templates/README.md b/railties/lib/rails/generators/rails/app/templates/README.md index 55e144da18..7db80e4ca1 100644 --- a/railties/lib/rails/generators/rails/app/templates/README.md +++ b/railties/lib/rails/generators/rails/app/templates/README.md @@ -1,4 +1,4 @@ -## README +# README This README would normally document whatever steps are necessary to get the application up and running. diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/cable.coffee b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/cable.coffee index 07934d026f..af08f58e34 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/cable.coffee +++ b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/cable.coffee @@ -1,11 +1,9 @@ # Action Cable provides the framework to deal with WebSockets in Rails. # You can generate new channels where WebSocket features live using the rails generate channel command. # -# Turn on the cable connection by removing the comments after the require statements (and ensure it's also on in config/routes.rb). -# #= require action_cable #= require_self #= require_tree ./channels -# -# @App ||= {} -# App.cable = ActionCable.createConsumer() + +@App ||= {} +App.cable = ActionCable.createConsumer() 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..72258cc96b 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 @@ -3,16 +3,13 @@ <head> <title><%= camelized %></title> <%%= csrf_meta_tags %> - <%- unless options[:skip_action_cable] -%> - <%%= action_cable_meta_tag %> - <%- end -%> <%- if options[:skip_javascript] -%> <%%= 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..e6a2de0928 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,18 +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 + config.cache_store = :memory_store config.public_file_server.headers = { 'Cache-Control' => 'public, max-age=172800' } else config.action_controller.perform_caching = false + config.cache_store = :null_store end <%- unless options.skip_action_mailer? -%> # Don't care if the mailer can't send. config.action_mailer.raise_delivery_errors = false + + config.action_mailer.perform_caching = false <%- end -%> # Print deprecation notices to the Rails logger. 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..0fc1179339 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. @@ -86,5 +91,9 @@ Rails.application.configure do # Do not dump schema after migrations. config.active_record.dump_schema_after_migration = false + + # Don't mount Action Cable in the main server process. + # config.action_cable.mount_path = nil + # config.action_cable.url = "ws://example.com" <%- end -%> end diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt index 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/generators/rails/app/templates/config/initializers/ssl_options.rb b/railties/lib/rails/generators/rails/app/templates/config/initializers/ssl_options.rb new file mode 100644 index 0000000000..1775dea1e7 --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/ssl_options.rb @@ -0,0 +1,4 @@ +# Be sure to restart your server when you modify this file. + +# Configure SSL options to enable HSTS with subdomains. +Rails.application.config.ssl_options = { hsts: { subdomains: true } } diff --git a/railties/lib/rails/generators/rails/app/templates/config/puma.rb b/railties/lib/rails/generators/rails/app/templates/config/puma.rb index 1bf274bc66..c7f311f811 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/puma.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/puma.rb @@ -42,3 +42,6 @@ environment ENV.fetch("RAILS_ENV") { "development" } # on_worker_boot do # ActiveRecord::Base.establish_connection if defined?(ActiveRecord) # end + +# Allow puma to be restarted by `rails restart` command. +plugin :tmp_restart diff --git a/railties/lib/rails/generators/rails/app/templates/config/routes.rb b/railties/lib/rails/generators/rails/app/templates/config/routes.rb index 8293c8a483..787824f888 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/routes.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/routes.rb @@ -1,6 +1,3 @@ Rails.application.routes.draw do # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html - - # Serve websocket cable requests in-process - # mount ActionCable.server => '/cable' end diff --git a/railties/lib/rails/test_unit/minitest_plugin.rb b/railties/lib/rails/test_unit/minitest_plugin.rb index efc8b82d61..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,7 +61,7 @@ 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 diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 383f485db5..d03dd1afcc 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 @@ -988,7 +999,7 @@ module ApplicationTests app 'development' post "/posts.json", '{ "title": "foo", "name": "bar" }', "CONTENT_TYPE" => "application/json" - assert_equal '<ActionController::Parameters {"title"=>"foo"}>', last_response.body + assert_equal '<ActionController::Parameters {"title"=>"foo"} permitted: false>', last_response.body end test "config.action_controller.permit_all_parameters = true" do 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/rake_test.rb b/railties/test/application/rake_test.rb index d1c828b509..3db467256e 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -118,7 +118,7 @@ module ApplicationTests end def test_code_statistics_sanity - assert_match "Code LOC: 14 Test LOC: 0 Code to Test Ratio: 1:0.0", + assert_match "Code LOC: 16 Test LOC: 0 Code to Test Ratio: 1:0.0", Dir.chdir(app_path){ `bin/rails stats` } 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/actions_test.rb b/railties/test/generators/actions_test.rb index 3300850604..6158748194 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -202,7 +202,7 @@ class ActionsTest < Rails::Generators::TestCase end def test_rake_should_run_rake_command_with_default_env - assert_called_with(generator, :run, ["rake log:clear RAILS_ENV=development", verbose: false]) do + assert_called_with(generator, :run, ["rails log:clear RAILS_ENV=development", verbose: false]) do with_rails_env nil do action :rake, 'log:clear' end @@ -210,13 +210,13 @@ class ActionsTest < Rails::Generators::TestCase end def test_rake_with_env_option_should_run_rake_command_in_env - assert_called_with(generator, :run, ['rake log:clear RAILS_ENV=production', verbose: false]) do + assert_called_with(generator, :run, ['rails log:clear RAILS_ENV=production', verbose: false]) do action :rake, 'log:clear', env: 'production' end end def test_rake_with_rails_env_variable_should_run_rake_command_in_env - assert_called_with(generator, :run, ['rake log:clear RAILS_ENV=production', verbose: false]) do + assert_called_with(generator, :run, ['rails log:clear RAILS_ENV=production', verbose: false]) do with_rails_env "production" do action :rake, 'log:clear' end @@ -224,7 +224,7 @@ class ActionsTest < Rails::Generators::TestCase end def test_env_option_should_win_over_rails_env_variable_when_running_rake - assert_called_with(generator, :run, ['rake log:clear RAILS_ENV=production', verbose: false]) do + assert_called_with(generator, :run, ['rails log:clear RAILS_ENV=production', verbose: false]) do with_rails_env "staging" do action :rake, 'log:clear', env: 'production' end @@ -232,13 +232,51 @@ class ActionsTest < Rails::Generators::TestCase end def test_rake_with_sudo_option_should_run_rake_command_with_sudo - assert_called_with(generator, :run, ["sudo rake log:clear RAILS_ENV=development", verbose: false]) do + assert_called_with(generator, :run, ["sudo rails log:clear RAILS_ENV=development", verbose: false]) do with_rails_env nil do action :rake, 'log:clear', sudo: true end end end + def test_rails_command_should_run_rails_command_with_default_env + assert_called_with(generator, :run, ["rails log:clear RAILS_ENV=development", verbose: false]) do + with_rails_env nil do + action :rails_command, 'log:clear' + end + end + end + + def test_rails_command_with_env_option_should_run_rails_command_in_env + assert_called_with(generator, :run, ['rails log:clear RAILS_ENV=production', verbose: false]) do + action :rails_command, 'log:clear', env: 'production' + end + end + + def test_rails_command_with_rails_env_variable_should_run_rails_command_in_env + assert_called_with(generator, :run, ['rails log:clear RAILS_ENV=production', verbose: false]) do + with_rails_env "production" do + action :rails_command, 'log:clear' + end + end + end + + def test_env_option_should_win_over_rails_env_variable_when_running_rails + assert_called_with(generator, :run, ['rails log:clear RAILS_ENV=production', verbose: false]) do + with_rails_env "staging" do + action :rails_command, 'log:clear', env: 'production' + end + end + end + + def test_rails_command_with_sudo_option_should_run_rails_command_with_sudo + assert_called_with(generator, :run, ["sudo rails log:clear RAILS_ENV=development", verbose: false]) do + with_rails_env nil do + action :rails_command, 'log:clear', sudo: true + end + end + end + def test_capify_should_run_the_capify_command assert_called_with(generator, :run, ['capify .', verbose: false]) do action :capify! diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 921a5b36b5..c5f9e11ad3 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 @@ -241,6 +241,34 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_rails_update_does_not_create_ssl_options_by_default + app_root = File.join(destination_root, 'myapp') + run_generator [app_root] + + FileUtils.rm("#{app_root}/config/initializers/ssl_options.rb") + + stub_rails_application(app_root) do + generator = Rails::Generators::AppGenerator.new ["rails"], { with_dispatchers: true }, destination_root: app_root, shell: @shell + generator.send(:app_const) + quietly { generator.send(:update_config_files) } + assert_no_file "#{app_root}/config/initializers/ssl_options.rb" + end + end + + def test_rails_update_does_not_remove_ssl_options_if_already_present + app_root = File.join(destination_root, 'myapp') + run_generator [app_root] + + FileUtils.touch("#{app_root}/config/initializers/ssl_options.rb") + + stub_rails_application(app_root) do + generator = Rails::Generators::AppGenerator.new ["rails"], { with_dispatchers: true }, destination_root: app_root, shell: @shell + generator.send(:app_const) + quietly { generator.send(:update_config_files) } + assert_file "#{app_root}/config/initializers/ssl_options.rb" + end + end + def test_application_names_are_not_singularized run_generator [File.join(destination_root, "hats")] assert_file "hats/config/environment.rb", /Rails\.application\.initialize!/ @@ -338,6 +366,11 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_generator_defaults_to_puma_version + run_generator [destination_root] + assert_gem "puma", "'~> 3.0'" + end + def test_generator_if_skip_puma_is_given run_generator [destination_root, "--skip-puma"] assert_no_file "config/puma.rb" @@ -406,9 +439,6 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_no_file "config/cable.yml" assert_no_file "app/assets/javascripts/cable.coffee" assert_no_file "app/channels" - assert_file "app/views/layouts/application.html.erb" do |content| - assert_no_match(/action_cable_meta_tag/, content) - end assert_file "Gemfile" do |content| assert_no_match(/redis/, content) 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) diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index d7d27e6b2e..cf3ed8405d 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -642,6 +642,20 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_file "app/models/bukkits/article.rb", /class Article < ApplicationRecord/ end + def test_generate_application_record_when_does_not_exist_in_mountable_engine + run_generator [destination_root, '--mountable'] + FileUtils.rm "#{destination_root}/app/models/bukkits/application_record.rb" + capture(:stdout) do + `#{destination_root}/bin/rails g model article` + end + + assert_file "#{destination_root}/app/models/bukkits/application_record.rb" do |record| + assert_match(/module Bukkits/, record) + assert_match(/class ApplicationRecord < ActiveRecord::Base/, record) + assert_match(/self.abstract_class = true/, record) + end + end + def test_after_bundle_callback path = 'http://example.org/rails_template' template = %{ after_bundle { run 'echo ran after_bundle' } } diff --git a/tools/README.md b/tools/README.md index 1f3d6c59d9..b2e7e4b0ae 100644 --- a/tools/README.md +++ b/tools/README.md @@ -1,8 +1,8 @@ -## Rails dev tools +# Rails dev tools This is a collection of utilities used for Rails internal development. They aren't used by Rails apps directly. * `console` drops you in irb and loads local Rails repos * `profile` profiles `Kernel#require` to help reduce startup time - * `line_statistics` provides CodeTools module and LineStatistics class to count lines
\ No newline at end of file + * `line_statistics` provides CodeTools module and LineStatistics class to count lines diff --git a/version.rb b/version.rb index 93e0151602..081222425c 100644 --- a/version.rb +++ b/version.rb @@ -8,7 +8,7 @@ module Rails MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end |