diff options
172 files changed, 1775 insertions, 714 deletions
diff --git a/.codeclimate.yml b/.codeclimate.yml index 139ee8013b..908a988e69 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -23,7 +23,7 @@ checks: engines: rubocop: enabled: true - channel: rubocop-0-60 + channel: rubocop-0-63 ratings: paths: diff --git a/.travis.yml b/.travis.yml index 8317b58c52..74f8d4795f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,3 +1,4 @@ +dist: xenial language: ruby sudo: false @@ -12,24 +13,20 @@ cache: services: - memcached - redis-server + - mysql addons: postgresql: 10 chrome: stable apt: sources: - - sourceline: "ppa:mc3man/trusty-media" + - sourceline: "ppa:jonathonf/ffmpeg-3" - sourceline: "ppa:ubuntuhandbook1/apps" - - mysql-5.7-trusty packages: - ffmpeg - mupdf - mupdf-tools - poppler-utils - - mysql-server - - mysql-client - - postgresql-10 - - postgresql-client-10 bundler_args: --jobs 3 --retry 3 before_install: @@ -43,6 +40,7 @@ before_install: - "[[ $GEM != 'actionview:ujs' ]] || (cd actionview && npm install)" - "[[ $GEM != 'railties' ]] || (curl -o- -L https://yarnpkg.com/install.sh | bash)" - "[[ $GEM != 'railties' ]] || export PATH=$HOME/.yarn/bin:$PATH" + - "[[ $GEM != 'activerecord:postgresql' ]] || sudo mount -o remount,size=50% /var/ramfs" before_script: # Set Sauce Labs username and access key. Obfuscated, purposefully not encrypted. @@ -56,10 +54,13 @@ env: global: - "JRUBY_OPTS='--dev -J-Xmx1024M'" matrix: + - "GEM=railties" - "GEM=actionpack,actioncable" - "GEM=actionmailer,activemodel,activesupport,actionview,activejob,activestorage,actionmailbox,actiontext" - "GEM=activesupport PRESERVE_TIMEZONES=1" - "GEM=activerecord:sqlite3" + - "GEM=activerecord:postgresql" + - "GEM=activerecord:mysql2" - "GEM=guides" - "GEM=actioncable:integration" @@ -71,126 +72,72 @@ rvm: matrix: include: - rvm: 2.5.3 - env: "GEM=railties" - sudo: required - before_install: - - "rm ${BUNDLE_GEMFILE}.lock" - - "travis_retry gem update --system" - - "travis_retry gem install bundler" - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - - rvm: 2.6.0 - env: "GEM=railties" - sudo: required - before_install: - - "rm ${BUNDLE_GEMFILE}.lock" - - "travis_retry gem update --system" - - "travis_retry gem install bundler" - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - - rvm: ruby-head - env: "GEM=railties" - sudo: required - before_install: - - "rm ${BUNDLE_GEMFILE}.lock" - - "travis_retry gem update --system" - - "travis_retry gem install bundler" - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - - rvm: 2.5.3 env: "GEM=actionview:ujs" - rvm: 2.5.3 sudo: required env: "GEM=activejob:integration" + addons: + postgresql: 10 + apt: + packages: + - rabbitmq-server services: - memcached - redis-server - - rabbitmq + - rabbitmq-server before_install: - - sudo sed -i -e '/local.*peer/s/postgres/all/' -e 's/peer\|md5/trust/g' /etc/postgresql/*/main/pg_hba.conf - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/beanstalkd/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" - "pushd /tmp/beanstalkd-1.10 && make && (./beanstalkd &); popd" - rvm: 2.6.0 sudo: required env: "GEM=activejob:integration" + addons: + postgresql: 10 + apt: + packages: + - rabbitmq-server services: - memcached - redis-server - - rabbitmq + - rabbitmq-server before_install: - - sudo sed -i -e '/local.*peer/s/postgres/all/' -e 's/peer\|md5/trust/g' /etc/postgresql/*/main/pg_hba.conf - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/beanstalkd/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" - "pushd /tmp/beanstalkd-1.10 && make && (./beanstalkd &); popd" - rvm: ruby-head sudo: required env: "GEM=activejob:integration" + addons: + postgresql: 10 + apt: + packages: + - rabbitmq-server services: - memcached - redis-server - - rabbitmq + - rabbitmq-server before_install: - - sudo sed -i -e '/local.*peer/s/postgres/all/' -e 's/peer\|md5/trust/g' /etc/postgresql/*/main/pg_hba.conf - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" + - "rm ${BUNDLE_GEMFILE}.lock" + - "travis_retry gem update --system" + - "travis_retry gem install bundler" - "[ -f /tmp/beanstalkd-1.10/Makefile ] || (curl -L https://github.com/beanstalkd/beanstalkd/archive/v1.10.tar.gz | tar xz -C /tmp)" - "pushd /tmp/beanstalkd-1.10 && make && (./beanstalkd &); popd" - rvm: 2.5.3 - env: "GEM=activerecord:mysql2" - sudo: required - before_install: - - "sudo mysql -e \"use mysql; update user set authentication_string='' where User='root'; update user set plugin='mysql_native_password';FLUSH PRIVILEGES;\"" - - "sudo mysql_upgrade" - - "sudo service mysql restart" - - rvm: 2.6.0 - env: "GEM=activerecord:mysql2" - sudo: required - before_install: - - "sudo mysql -e \"use mysql; update user set authentication_string='' where User='root'; update user set plugin='mysql_native_password';FLUSH PRIVILEGES;\"" - - "sudo mysql_upgrade" - - "sudo service mysql restart" - - rvm: ruby-head - env: "GEM=activerecord:mysql2" - sudo: required + env: + - "GEM=activerecord:mysql2 MYSQL=mariadb" before_install: - - "sudo mysql -e \"use mysql; update user set authentication_string='' where User='root'; update user set plugin='mysql_native_password';FLUSH PRIVILEGES;\"" - "sudo mysql_upgrade" - "sudo service mysql restart" - - rvm: 2.5.3 - env: - - "GEM=activerecord:mysql2 MYSQL=mariadb" addons: mariadb: 10.3 - rvm: 2.5.3 env: - "GEM=activerecord:sqlite3_mem" - - rvm: 2.5.3 - env: "GEM=activerecord:postgresql" - sudo: required - before_install: - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - - rvm: 2.6.0 - env: "GEM=activerecord:postgresql" - sudo: required - before_install: - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - - rvm: ruby-head - env: "GEM=activerecord:postgresql" - sudo: required - before_install: - - "sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/10/main/postgresql.conf" - - "sudo service postgresql restart 10" - rvm: jruby-head - jdk: oraclejdk8 + jdk: oraclejdk11 env: - "GEM=actionpack" - rvm: jruby-head - jdk: oraclejdk8 + jdk: oraclejdk11 env: - "GEM=actionmailer,activemodel,activejob" allow_failures: @@ -10,6 +10,7 @@ gemspec gem "rake", ">= 11.1" gem "capybara", ">= 2.15" +gem "selenium-webdriver", ">= 3.5.0", "< 3.13.0" gem "rack-cache", "~> 1.2" gem "sass-rails" @@ -52,7 +53,7 @@ group :job do gem "sidekiq", require: false gem "sucker_punch", require: false gem "delayed_job", require: false - gem "queue_classic", github: "rafaelfranca/queue_classic", branch: "update-pg", require: false, platforms: :ruby + gem "queue_classic", github: "QueueClassic/queue_classic", require: false, platforms: :ruby gem "sneakers", require: false gem "que", require: false gem "backburner", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 8062fdb695..996e9d5942 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,4 +1,11 @@ GIT + remote: https://github.com/QueueClassic/queue_classic.git + revision: 4cdc9b8e804badf7dea7078dd81092972d292c14 + specs: + queue_classic (3.2.0.RC1) + pg (>= 0.17, < 2.0) + +GIT remote: https://github.com/matthewd/websocket-client-simple.git revision: e161305f1a466b9398d86df3b1731b03362da91b branch: close-race @@ -7,14 +14,6 @@ GIT event_emitter websocket -GIT - remote: https://github.com/rafaelfranca/queue_classic.git - revision: dee64b361355d56700ad7aa3b151bf653a617526 - branch: update-pg - specs: - queue_classic (3.2.0.RC1) - pg (>= 0.17, < 2.0) - PATH remote: . specs: @@ -172,8 +171,8 @@ GEM bootsnap (1.3.2-java) msgpack (~> 1.0) builder (3.2.3) - bunny (2.9.2) - amq-protocol (~> 2.3.0) + bunny (2.13.0) + amq-protocol (~> 2.3, >= 2.3.0) byebug (10.0.2) capybara (3.10.1) addressable @@ -192,7 +191,7 @@ GEM coffee-script-source execjs coffee-script-source (1.12.2) - concurrent-ruby (1.1.3) + concurrent-ruby (1.1.4) connection_pool (2.2.2) cookiejar (0.3.3) crack (0.4.3) @@ -239,10 +238,10 @@ GEM faye-websocket (0.10.7) eventmachine (>= 0.12.0) websocket-driver (>= 0.5.1) - ffi (1.9.25) - ffi (1.9.25-java) - ffi (1.9.25-x64-mingw32) - ffi (1.9.25-x86-mingw32) + ffi (1.10.0) + ffi (1.10.0-java) + ffi (1.10.0-x64-mingw32) + ffi (1.10.0-x86-mingw32) fugit (1.1.6) et-orbi (~> 1.1, >= 1.1.6) raabro (~> 1.1) @@ -283,8 +282,8 @@ GEM mini_magick (~> 4.0) ruby-vips (>= 2.0.13, < 3) io-like (0.3.0) - jaro_winkler (1.5.1) - jaro_winkler (1.5.1-java) + jaro_winkler (1.5.2) + jaro_winkler (1.5.2-java) jdbc-mysql (5.1.46) jdbc-postgres (42.1.4) jdbc-sqlite3 (3.20.1) @@ -337,6 +336,7 @@ GEM mysql2 (0.5.2-x64-mingw32) mysql2 (0.5.2-x86-mingw32) nio4r (2.3.1) + nio4r (2.3.1-java) nokogiri (1.9.1) mini_portile2 (~> 2.4.0) nokogiri (1.9.1-java) @@ -345,8 +345,8 @@ GEM nokogiri (1.9.1-x86-mingw32) mini_portile2 (~> 2.4.0) os (1.0.0) - parallel (1.12.1) - parser (2.5.3.0) + parallel (1.13.0) + parser (2.6.0.0) ast (~> 2.4.0) path_expander (1.0.3) pg (1.1.3) @@ -404,7 +404,7 @@ GEM resque (~> 1.26) rufus-scheduler (~> 3.2) retriable (3.1.2) - rubocop (0.61.1) + rubocop (0.63.0) jaro_winkler (~> 1.5.1) parallel (~> 1.10) parser (>= 2.5, != 2.5.1.1) @@ -433,9 +433,9 @@ GEM tilt (>= 1.1, < 3) sdoc (1.0.0) rdoc (>= 5.0) - selenium-webdriver (3.141.0) + selenium-webdriver (3.12.0) childprocess (~> 0.5) - rubyzip (~> 1.2, >= 1.2.2) + rubyzip (~> 1.2) sequel (5.14.0) serverengine (2.0.7) sigdump (~> 0.2.2) @@ -454,9 +454,10 @@ GEM rack (~> 2.0) rack-protection (= 2.0.4) tilt (~> 2.0) - sneakers (2.7.0) - bunny (~> 2.9.2) + sneakers (2.11.0) + bunny (~> 2.12) concurrent-ruby (~> 1.0) + rake serverengine (~> 2.0.5) thor sprockets (3.7.2) @@ -510,6 +511,8 @@ GEM websocket (1.2.8) websocket-driver (0.7.0) websocket-extensions (>= 0.1.0) + websocket-driver (0.7.0-java) + websocket-extensions (>= 0.1.0) websocket-extensions (0.1.3) xpath (3.2.0) nokogiri (~> 1.8) @@ -569,6 +572,7 @@ DEPENDENCIES rubocop (>= 0.47) sass-rails sdoc (~> 1.0) + selenium-webdriver (>= 3.5.0, < 3.13.0) sequel sidekiq sneakers diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md index 4109dd6138..42edbd6954 100644 --- a/actioncable/CHANGELOG.md +++ b/actioncable/CHANGELOG.md @@ -1,5 +1,43 @@ +* Add `:action_cable_connection` and `:action_cable_channel` load hooks. + + You can use them to extend `ActionCable::Connection::Base` and `ActionCable::Channel::Base` + functionality: + + ```ruby + ActiveSupport.on_load(:action_cable_channel) do + # do something in the context of ActionCable::Channel::Base + end + ``` + + *Vladimir Dementyev* + +* Add `Channel::Base#broadcast_to`. + + You can now call `broadcast_to` within a channel action, which equals to + the `self.class.broadcast_to`. + + *Vladimir Dementyev* + +* Make `Channel::Base.broadcasting_for` a public API. + + You can use `.broadcasting_for` to generate a unique stream identifier within + a channel for the specified target (e.g. Active Record model): + + ```ruby + ChatChannel.broadcasting_for(model) # => "chat:<model.to_gid_param>" + ``` + + *Vladimir Dementyev* + + ## Rails 6.0.0.beta1 (January 18, 2019) ## +* [Rename npm package](https://github.com/rails/rails/pull/34905) from + [`actioncable`](https://www.npmjs.com/package/actioncable) to + [`@rails/actioncable`](https://www.npmjs.com/package/@rails/actioncable). + + *Javan Makhmali* + * Merge [`action-cable-testing`](https://github.com/palkan/action-cable-testing) to Rails. *Vladimir Dementyev* diff --git a/actioncable/lib/action_cable/channel/base.rb b/actioncable/lib/action_cable/channel/base.rb index ad0d3685cd..af061c843a 100644 --- a/actioncable/lib/action_cable/channel/base.rb +++ b/actioncable/lib/action_cable/channel/base.rb @@ -307,3 +307,5 @@ module ActionCable end end end + +ActiveSupport.run_load_hooks(:action_cable_channel, ActionCable::Channel::Base) diff --git a/actioncable/lib/action_cable/channel/broadcasting.rb b/actioncable/lib/action_cable/channel/broadcasting.rb index 9a96720f4a..9f702e425e 100644 --- a/actioncable/lib/action_cable/channel/broadcasting.rb +++ b/actioncable/lib/action_cable/channel/broadcasting.rb @@ -7,22 +7,32 @@ module ActionCable module Broadcasting extend ActiveSupport::Concern - delegate :broadcasting_for, to: :class + delegate :broadcasting_for, :broadcast_to, to: :class module ClassMethods # Broadcast a hash to a unique broadcasting for this <tt>model</tt> in this channel. def broadcast_to(model, message) - ActionCable.server.broadcast(broadcasting_for([ channel_name, model ]), message) + ActionCable.server.broadcast(broadcasting_for(model), message) end - def broadcasting_for(model) #:nodoc: + # Returns a unique broadcasting identifier for this <tt>model</tt> in this channel: + # + # CommentsChannel.broadcasting_for("all") # => "comments:all" + # + # You can pass any object as a target (e.g. Active Record model), and it + # would be serialized into a string under the hood. + def broadcasting_for(model) + serialize_broadcasting([ channel_name, model ]) + end + + def serialize_broadcasting(object) #:nodoc: case - when model.is_a?(Array) - model.map { |m| broadcasting_for(m) }.join(":") - when model.respond_to?(:to_gid_param) - model.to_gid_param + when object.is_a?(Array) + object.map { |m| serialize_broadcasting(m) }.join(":") + when object.respond_to?(:to_gid_param) + object.to_gid_param else - model.to_param + object.to_param end end end diff --git a/actioncable/lib/action_cable/channel/streams.rb b/actioncable/lib/action_cable/channel/streams.rb index 81c2c38064..7e1ed3c850 100644 --- a/actioncable/lib/action_cable/channel/streams.rb +++ b/actioncable/lib/action_cable/channel/streams.rb @@ -99,7 +99,7 @@ module ActionCable # Pass <tt>coder: ActiveSupport::JSON</tt> to decode messages as JSON before passing to the callback. # Defaults to <tt>coder: nil</tt> which does no decoding, passes raw messages. def stream_for(model, callback = nil, coder: nil, &block) - stream_from(broadcasting_for([ channel_name, model ]), callback || block, coder: coder) + stream_from(broadcasting_for(model), callback || block, coder: coder) end # Unsubscribes all streams associated with this channel from the pubsub queue. diff --git a/actioncable/lib/action_cable/channel/test_case.rb b/actioncable/lib/action_cable/channel/test_case.rb index c4cf0ac0e7..b05d51a61a 100644 --- a/actioncable/lib/action_cable/channel/test_case.rb +++ b/actioncable/lib/action_cable/channel/test_case.rb @@ -143,7 +143,7 @@ module ActionCable # You need to set up your connection manually to provide values for the identifiers. # To do this just use: # - # stub_connection(user: users[:john]) + # stub_connection(user: users(:john)) # # == Testing broadcasting # @@ -157,9 +157,9 @@ module ActionCable # end # # def test_speak - # subscribe room_id: rooms[:chat].id + # subscribe room_id: rooms(:chat).id # - # assert_broadcasts_on(rooms[:chat], text: "Hello, Rails!") do + # assert_broadcasts_on(rooms(:chat), text: "Hello, Rails!") do # perform :speak, message: "Hello, Rails!" # end # end @@ -300,9 +300,7 @@ module ActionCable def broadcasting_for(stream_or_object) return stream_or_object if stream_or_object.is_a?(String) - self.class.channel_class.broadcasting_for( - [self.class.channel_class.channel_name, stream_or_object] - ) + self.class.channel_class.broadcasting_for(stream_or_object) end end diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb index 0044afad98..c469f7066c 100644 --- a/actioncable/lib/action_cable/connection/base.rb +++ b/actioncable/lib/action_cable/connection/base.rb @@ -260,3 +260,5 @@ module ActionCable end end end + +ActiveSupport.run_load_hooks(:action_cable_connection, ActionCable::Connection::Base) diff --git a/actioncable/lib/action_cable/connection/test_case.rb b/actioncable/lib/action_cable/connection/test_case.rb index 26a183d1ec..8d25a55c8a 100644 --- a/actioncable/lib/action_cable/connection/test_case.rb +++ b/actioncable/lib/action_cable/connection/test_case.rb @@ -42,8 +42,6 @@ module ActionCable class TestRequest < ActionDispatch::TestRequest attr_accessor :session, :cookie_jar - - attr_writer :cookie_jar end module TestConnection diff --git a/actioncable/test/channel/broadcasting_test.rb b/actioncable/test/channel/broadcasting_test.rb index 2cbfabc1d0..fb501a1bc2 100644 --- a/actioncable/test/channel/broadcasting_test.rb +++ b/actioncable/test/channel/broadcasting_test.rb @@ -26,14 +26,23 @@ class ActionCable::Channel::BroadcastingTest < ActionCable::TestCase end test "broadcasting_for with an object" do - assert_equal "Room#1-Campfire", ChatChannel.broadcasting_for(Room.new(1)) + assert_equal( + "action_cable:channel:broadcasting_test:chat:Room#1-Campfire", + ChatChannel.broadcasting_for(Room.new(1)) + ) end test "broadcasting_for with an array" do - assert_equal "Room#1-Campfire:Room#2-Campfire", ChatChannel.broadcasting_for([ Room.new(1), Room.new(2) ]) + assert_equal( + "action_cable:channel:broadcasting_test:chat:Room#1-Campfire:Room#2-Campfire", + ChatChannel.broadcasting_for([ Room.new(1), Room.new(2) ]) + ) end test "broadcasting_for with a string" do - assert_equal "hello", ChatChannel.broadcasting_for("hello") + assert_equal( + "action_cable:channel:broadcasting_test:chat:hello", + ChatChannel.broadcasting_for("hello") + ) end end diff --git a/actioncable/test/channel/test_case_test.rb b/actioncable/test/channel/test_case_test.rb index 9c360d5dc3..a166c41e11 100644 --- a/actioncable/test/channel/test_case_test.rb +++ b/actioncable/test/channel/test_case_test.rb @@ -180,7 +180,7 @@ class BroadcastsTestChannel < ActionCable::Channel::Base def broadcast_to_user(data) user = User.new user_id - self.class.broadcast_to user, text: data["message"] + broadcast_to user, text: data["message"] end end diff --git a/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb b/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb index 2cde3db8a0..d051dfe665 100644 --- a/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb +++ b/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb @@ -22,7 +22,7 @@ module Rails def new_mail Mail.new(params.require(:mail).permit(:from, :to, :cc, :bcc, :in_reply_to, :subject, :body).to_h).tap do |mail| params[:mail][:attachments].to_a.each do |attachment| - mail.attachments[attachment.original_filename] = { filename: attachment.path, content_type: attachment.content_type } + mail.add_file(filename: attachment.path, content: attachment.read) end end end diff --git a/actionmailbox/app/views/layouts/rails/conductor.html.erb b/actionmailbox/app/views/layouts/rails/conductor.html.erb index 75157feb78..1cad6560c4 100644 --- a/actionmailbox/app/views/layouts/rails/conductor.html.erb +++ b/actionmailbox/app/views/layouts/rails/conductor.html.erb @@ -4,4 +4,5 @@ </head> <body> <%= yield %> +</body> </html> diff --git a/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb b/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb index 89ab66c1a9..2bf4335808 100644 --- a/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb +++ b/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb @@ -5,11 +5,7 @@ class CreateActionMailboxTables < ActiveRecord::Migration[6.0] t.string :message_id, null: false t.string :message_checksum, null: false - if supports_datetime_with_precision? - t.timestamps precision: 6 - else - t.timestamps - end + t.timestamps t.index [ :message_id, :message_checksum ], name: "index_action_mailbox_inbound_emails_uniqueness", unique: true end diff --git a/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb b/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb index 60077d86e2..fcd9ad839f 100644 --- a/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb +++ b/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb @@ -10,16 +10,39 @@ class Rails::Conductor::ActionMailbox::InboundEmailsControllerTest < ActionDispa mail: { from: "Jason Fried <jason@37signals.com>", to: "Replies <replies@example.com>", - bcc: "", in_reply_to: "<4e6e35f5a38b4_479f13bb90078178@small-app-01.mail>", - subject: "Discussion: Let's debate these attachments", + subject: "Hey there", + body: "How's it going?" + } + } + end + + mail = ActionMailbox::InboundEmail.last.mail + assert_equal %w[ jason@37signals.com ], mail.from + assert_equal %w[ replies@example.com ], mail.to + assert_equal "4e6e35f5a38b4_479f13bb90078178@small-app-01.mail", mail.in_reply_to + assert_equal "Hey there", mail.subject + assert_equal "How's it going?", mail.body.decoded + end + end + + test "create inbound email with attachments" do + with_rails_env("development") do + assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do + post rails_conductor_inbound_emails_path, params: { + mail: { + from: "Jason Fried <jason@37signals.com>", + to: "Replies <replies@example.com>", + subject: "Let's debate some attachments", body: "Let's talk about these images:", - attachments: [fixture_file_upload("files/avatar1.jpeg"), fixture_file_upload("files/avatar2.jpeg")] + attachments: [ fixture_file_upload("files/avatar1.jpeg"), fixture_file_upload("files/avatar2.jpeg") ] } } end - assert_equal 2, ActionMailbox::InboundEmail.last.mail.attachments.size + mail = ActionMailbox::InboundEmail.last.mail + assert_equal "Let's talk about these images:", mail.text_part.decoded + assert_equal 2, mail.attachments.count end end diff --git a/actionmailbox/test/dummy/config/puma.rb b/actionmailbox/test/dummy/config/puma.rb index a5eccf816b..71ed69a895 100644 --- a/actionmailbox/test/dummy/config/puma.rb +++ b/actionmailbox/test/dummy/config/puma.rb @@ -16,7 +16,7 @@ port ENV.fetch("PORT") { 3000 } environment ENV.fetch("RAILS_ENV") { "development" } # Specifies the number of `workers` to boot in clustered mode. -# Workers are forked webserver processes. If using threads and workers together +# Workers are forked web server processes. If using threads and workers together # the concurrency of the application would be max `threads` * `workers`. # Workers do not work on JRuby or Windows (both of which do not support # processes). diff --git a/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailbox_tables.rb b/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailbox_tables.rb index 89ab66c1a9..2bf4335808 100644 --- a/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailbox_tables.rb +++ b/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailbox_tables.rb @@ -5,11 +5,7 @@ class CreateActionMailboxTables < ActiveRecord::Migration[6.0] t.string :message_id, null: false t.string :message_checksum, null: false - if supports_datetime_with_precision? - t.timestamps precision: 6 - else - t.timestamps - end + t.timestamps t.index [ :message_id, :message_checksum ], name: "index_action_mailbox_inbound_emails_uniqueness", unique: true end diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 1a1b1034aa..d9041aecb7 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,9 @@ +* `ActionDispatch::SystemTestCase.driven_by` can now be called with a block + to define specific browser capabilities. + + *Edouard Chin* + + ## Rails 6.0.0.beta1 (January 18, 2019) ## * Remove deprecated `fragment_cache_key` helper in favor of `combined_fragment_cache_key`. @@ -11,12 +17,6 @@ *Rafael Mendonça França* -* Ensure external redirects are explicitly allowed - - Add `fallback_location` and `allow_other_host` options to `redirect_to`. - - *Gannon McGibbon* - * Introduce ActionDispatch::HostAuthorization This is a new middleware that guards against DNS rebinding attacks by @@ -82,7 +82,7 @@ * Apply mapping to symbols returned from dynamic CSP sources Previously if a dynamic source returned a symbol such as :self it - would be converted to a string implicity, e.g: + would be converted to a string implicitly, e.g: policy.default_src -> { :self } @@ -135,7 +135,7 @@ *Assain Jaleel* -* Raises `ActionController::RespondToMismatchError` with confliciting `respond_to` invocations. +* Raises `ActionController::RespondToMismatchError` with conflicting `respond_to` invocations. `respond_to` can match multiple types and lead to undefined behavior when multiple invocations are made and the types do not match: @@ -160,7 +160,7 @@ *Aaron Kromer* -* Pass along arguments to underlying `get` method in `follow_redirect!`. +* Pass along arguments to underlying `get` method in `follow_redirect!` Now all arguments passed to `follow_redirect!` are passed to the underlying `get` method. This for example allows to set custom headers for the diff --git a/actionpack/lib/abstract_controller/caching/fragments.rb b/actionpack/lib/abstract_controller/caching/fragments.rb index 4e454adc5f..18677ddd18 100644 --- a/actionpack/lib/abstract_controller/caching/fragments.rb +++ b/actionpack/lib/abstract_controller/caching/fragments.rb @@ -28,7 +28,6 @@ module AbstractController self.fragment_cache_keys = [] if respond_to?(:helper_method) - helper_method :fragment_cache_key helper_method :combined_fragment_cache_key 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 2dc990f303..f9a758ff0e 100644 --- a/actionpack/lib/action_controller/metal/basic_implicit_render.rb +++ b/actionpack/lib/action_controller/metal/basic_implicit_render.rb @@ -6,7 +6,7 @@ module ActionController super.tap { default_render unless performed? } end - def default_render(*args) + def default_render head :no_content end end diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb index 30034be018..e1e0c6f456 100644 --- a/actionpack/lib/action_controller/metal/exceptions.rb +++ b/actionpack/lib/action_controller/metal/exceptions.rb @@ -52,7 +52,7 @@ module ActionController end # Raised when a nested respond_to is triggered and the content types of each - # are incompatible. For exampe: + # are incompatible. For example: # # respond_to do |outer_type| # outer_type.js do diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb index 205f84ae36..93fd57b640 100644 --- a/actionpack/lib/action_controller/metal/force_ssl.rb +++ b/actionpack/lib/action_controller/metal/force_ssl.rb @@ -13,7 +13,7 @@ module ActionController ACTION_OPTIONS = [:only, :except, :if, :unless] URL_OPTIONS = [:protocol, :host, :domain, :subdomain, :port, :path] - REDIRECT_OPTIONS = [:status, :flash, :alert, :notice, :allow_other_host] + REDIRECT_OPTIONS = [:status, :flash, :alert, :notice] module ClassMethods # :nodoc: def force_ssl(options = {}) @@ -41,7 +41,6 @@ module ActionController host: request.host, path: request.fullpath, status: :moved_permanently, - allow_other_host: true, } if host_or_options.is_a?(Hash) diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index 0faaac1ce4..f1fb7ab0f7 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -75,7 +75,7 @@ module ActionController # Provides a proxy to access helper methods from outside the view. def helpers @helper_proxy ||= begin - proxy = ActionView::Base.new + proxy = ActionView::Base.empty proxy.config = config.inheritable_copy proxy.extend(_helpers) end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index d3bb58f48b..8365ddca57 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -30,9 +30,9 @@ module ActionController # :stopdoc: include BasicImplicitRender - def default_render(*args) + def default_render if template_exists?(action_name.to_s, _prefixes, variants: request.variant) - render(*args) + render elsif any_templates?(action_name.to_s, _prefixes) message = "#{self.class.name}\##{action_name} is missing a template " \ "for this request format and variant.\n" \ diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 8bd003f5ed..67c198d150 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -60,7 +60,7 @@ module ActionController raise AbstractController::DoubleRenderError if response_body self.status = _extract_redirect_to_status(options, response_options) - self.location = _compute_safe_redirect_to_location(request, options, response_options) + self.location = _compute_redirect_to_location(request, options) self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>" end @@ -88,13 +88,9 @@ module ActionController # All other options that can be passed to <tt>redirect_to</tt> are accepted as # options and the behavior is identical. def redirect_back(fallback_location:, allow_other_host: true, **args) - referer = request.headers.fetch("Referer", fallback_location) - response_options = { - fallback_location: fallback_location, - allow_other_host: allow_other_host, - **args, - } - redirect_to referer, response_options + referer = request.headers["Referer"] + redirect_to_referer = referer && (allow_other_host || _url_host_allowed?(referer)) + redirect_to redirect_to_referer ? referer : fallback_location, **args end def _compute_redirect_to_location(request, options) #:nodoc: @@ -118,23 +114,6 @@ module ActionController public :_compute_redirect_to_location private - def _compute_safe_redirect_to_location(request, options, response_options) - location = _compute_redirect_to_location(request, options) - location_options = options.is_a?(Hash) ? options : {} - if response_options[:allow_other_host] || _url_host_allowed?(location, location_options) - location - else - fallback_location = response_options.fetch(:fallback_location) do - raise ArgumentError, <<~MSG.squish - Unsafe redirect #{location.inspect}, - use :fallback_location to specify a fallback - or :allow_other_host to redirect anyway. - MSG - end - _compute_redirect_to_location(request, fallback_location) - end - end - def _extract_redirect_to_status(options, response_options) if options.is_a?(Hash) && options.key?(:status) Rack::Utils.status_code(options.delete(:status)) @@ -145,8 +124,8 @@ module ActionController end end - def _url_host_allowed?(url, options = {}) - URI(url.to_s).host.in?([request.host, options[:host]]) + def _url_host_allowed?(url) + URI(url.to_s).host == request.host rescue ArgumentError, URI::Error false end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 04922b0715..815f82a1f2 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -795,7 +795,7 @@ module ActionController @permitted = coder.map["ivars"][:@permitted] when "!ruby/object:ActionController::Parameters" # YAML's Object format. Only needed because of the format - # backwardscompability above, otherwise equivalent to YAML's initialization. + # backwards compatibility above, otherwise equivalent to YAML's initialization. @parameters, @permitted = coder.map["parameters"], coder.map["permitted"] end end diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb index ac12dc13a1..5a7010a1c2 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_view.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -10,7 +10,9 @@ module ActionDispatch RESCUES_TEMPLATE_PATH = File.expand_path("templates", __dir__) def initialize(assigns) - super([RESCUES_TEMPLATE_PATH], assigns) + paths = [RESCUES_TEMPLATE_PATH] + renderer = ActionView::Renderer.new ActionView::LookupContext.new(paths) + super(renderer, assigns) end def debug_params(params) diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index fb2b2bd3b0..1fb3e9db00 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -31,22 +31,34 @@ module ActionDispatch "ActionController::MissingExactTemplate" => "missing_exact_template", ) + cattr_accessor :wrapper_exceptions, default: [ + "ActionView::Template::Error" + ] + attr_reader :backtrace_cleaner, :exception, :wrapped_causes, :line_number, :file def initialize(backtrace_cleaner, exception) @backtrace_cleaner = backtrace_cleaner - @exception = original_exception(exception) + @exception = exception @wrapped_causes = wrapped_causes_for(exception, backtrace_cleaner) expand_backtrace if exception.is_a?(SyntaxError) || exception.cause.is_a?(SyntaxError) end + def unwrapped_exception + if wrapper_exceptions.include?(exception.class.to_s) + exception.cause + else + exception + end + end + def rescue_template @@rescue_templates[@exception.class.name] end def status_code - self.class.status_code_for_exception(@exception.class.name) + self.class.status_code_for_exception(unwrapped_exception.class.name) end def application_trace @@ -122,14 +134,6 @@ module ActionDispatch Array(@exception.backtrace) end - def original_exception(exception) - if @@rescue_responses.has_key?(exception.cause.class.name) - exception.cause - else - exception - end - end - def causes_for(exception) return enum_for(__method__, exception) unless block_given? diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 3c88afd4d3..767143a368 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -45,7 +45,7 @@ module ActionDispatch backtrace_cleaner = request.get_header "action_dispatch.backtrace_cleaner" wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) status = wrapper.status_code - request.set_header "action_dispatch.exception", wrapper.exception + request.set_header "action_dispatch.exception", wrapper.unwrapped_exception request.set_header "action_dispatch.original_path", request.path_info request.path_info = "/#{status}" response = @exceptions_app.call(request.env) diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb index 2fa78dd385..1fbc107e28 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb @@ -2,6 +2,6 @@ <h1>Blocked host: <%= @host %></h1> </header> <div id="container"> - <h2>To allow requests to <%= @host %>, add the following configuration:</h2> - <pre>Rails.application.config.hosts << "<%= @host %>"</pre> + <h2>To allow requests to <%= @host %>, add the following to your environment configuration:</h2> + <pre>config.hosts << "<%= @host %>"</pre> </div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb index 4e2d1d0b08..a94dd982a7 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb @@ -1,5 +1,5 @@ Blocked host: <%= @host %> -To allow requests to <%= @host %>, add the following configuration: +To allow requests to <%= @host %>, add the following to your environment configuration: - Rails.application.config.hosts << "<%= @host %>" + config.hosts << "<%= @host %>" diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb index c74c0ccced..484eb46331 100644 --- a/actionpack/lib/action_dispatch/system_test_case.rb +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -89,6 +89,24 @@ module ActionDispatch # { js_errors: true } # end # + # Some drivers require browser capabilities to be passed as a block instead + # of through the +options+ hash. + # + # As an example, if you want to add mobile emulation on chrome, you'll have to + # create an instance of selenium's `Chrome::Options` object and add + # capabilities with a block. + # + # The block will be passed an instance of `<Driver>::Options` where you can + # define the capabilities you want. Please refer to your driver documentation + # to learn about supported options. + # + # class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + # driven_by :chrome, screen_size: [1024, 768] do |driver_option| + # driver_option.add_emulation(device: 'iPhone 6') + # driver_option.add_extension('path/to/chrome_extension.crx') + # end + # end + # # Because <tt>ActionDispatch::SystemTestCase</tt> is a shim between Capybara # and Rails, any driver that is supported by Capybara is supported by system # tests as long as you include the required gems and files. @@ -134,8 +152,10 @@ module ActionDispatch # driven_by :selenium, using: :firefox # # driven_by :selenium, using: :headless_firefox - def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400], options: {}) - self.driver = SystemTesting::Driver.new(driver, using: using, screen_size: screen_size, options: options) + def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400], options: {}, &capabilities) + driver_options = { using: using, screen_size: screen_size, options: options } + + self.driver = SystemTesting::Driver.new(driver, driver_options, &capabilities) end driven_by :selenium diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb index 1b0bce6b9e..c34907b6cb 100644 --- a/actionpack/lib/action_dispatch/system_testing/browser.rb +++ b/actionpack/lib/action_dispatch/system_testing/browser.rb @@ -29,20 +29,28 @@ module ActionDispatch end end + def capabilities + @option ||= + case type + when :chrome + ::Selenium::WebDriver::Chrome::Options.new + when :firefox + ::Selenium::WebDriver::Firefox::Options.new + end + end + private def headless_chrome_browser_options - options = Selenium::WebDriver::Chrome::Options.new - options.args << "--headless" - options.args << "--disable-gpu" if Gem.win_platform? + capabilities.args << "--headless" + capabilities.args << "--disable-gpu" if Gem.win_platform? - options + capabilities end def headless_firefox_browser_options - options = Selenium::WebDriver::Firefox::Options.new - options.args << "-headless" + capabilities.args << "-headless" - options + capabilities end end end diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb index 5252ff6746..25a09dd918 100644 --- a/actionpack/lib/action_dispatch/system_testing/driver.rb +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -3,11 +3,12 @@ module ActionDispatch module SystemTesting class Driver # :nodoc: - def initialize(name, **options) + def initialize(name, **options, &capabilities) @name = name @browser = Browser.new(options[:using]) @screen_size = options[:screen_size] @options = options[:options] + @capabilities = capabilities end def use @@ -22,6 +23,8 @@ module ActionDispatch end def register + define_browser_capabilities(@browser.capabilities) + Capybara.register_driver @name do |app| case @name when :selenium then register_selenium(app) @@ -31,6 +34,10 @@ module ActionDispatch end end + def define_browser_capabilities(capabilities) + @capabilities.call(capabilities) if @capabilities + end + def browser_options @options.merge(options: @browser.options).compact end diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index af41521c5c..28cde6704e 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -160,9 +160,16 @@ module ActionDispatch @controller.singleton_class.include(_routes.url_helpers) if @controller.respond_to? :view_context_class - @controller.view_context_class = Class.new(@controller.view_context_class) do + view_context_class = Class.new(@controller.view_context_class) do include _routes.url_helpers end + + custom_view_context = Module.new { + define_method(:view_context_class) do + view_context_class + end + } + @controller.extend(custom_view_context) end end yield @routes diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index c7aae034dd..ecb8c37e6b 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -28,13 +28,13 @@ class ActionPackAssertionsController < ActionController::Base def redirect_to_path() redirect_to "/some/path" end - def redirect_invalid_external_route() redirect_to "ht_tp://www.rubyonrails.org", allow_other_host: true end + def redirect_invalid_external_route() redirect_to "ht_tp://www.rubyonrails.org" end def redirect_to_named_route() redirect_to route_one_url end - def redirect_external() redirect_to "http://www.rubyonrails.org", allow_other_host: true; end + def redirect_external() redirect_to "http://www.rubyonrails.org"; end - def redirect_external_protocol_relative() redirect_to "//www.rubyonrails.org", allow_other_host: true; end + def redirect_external_protocol_relative() redirect_to "//www.rubyonrails.org"; end def response404() head "404 AWOL" end diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index cbebc6b59c..0562c16284 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -25,11 +25,11 @@ module Another end def redirector - redirect_to "http://foo.bar/", allow_other_host: true + redirect_to "http://foo.bar/" end def filterable_redirector - redirect_to "http://secret.foo.bar/", allow_other_host: true + redirect_to "http://secret.foo.bar/" end def data_sender diff --git a/actionpack/test/controller/new_base/render_context_test.rb b/actionpack/test/controller/new_base/render_context_test.rb deleted file mode 100644 index 5e570a1d79..0000000000 --- a/actionpack/test/controller/new_base/render_context_test.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require "abstract_unit" - -# This is testing the decoupling of view renderer and view context -# by allowing the controller to be used as view context. This is -# similar to the way sinatra renders templates. -module RenderContext - class BasicController < ActionController::Base - self.view_paths = [ActionView::FixtureResolver.new( - "render_context/basic/hello_world.html.erb" => "<%= @value %> from <%= self.__controller_method__ %>", - "layouts/basic.html.erb" => "?<%= yield %>?" - )] - - # 1) Include ActionView::Context to bring the required dependencies - include ActionView::Context - - # 2) Call _prepare_context that will do the required initialization - before_action :_prepare_context - - def hello_world - @value = "Hello" - render action: "hello_world", layout: false - end - - def with_layout - @value = "Hello" - render action: "hello_world", layout: "basic" - end - - protected def __controller_method__ - "controller context!" - end - - private - # 3) Set view_context to self - def view_context - self - end - end - - class RenderContextTest < Rack::TestCase - test "rendering using the controller as context" do - get "/render_context/basic/hello_world" - assert_body "Hello from controller context!" - assert_status 200 - end - - test "rendering using the controller as context with layout" do - get "/render_context/basic/with_layout" - assert_body "?Hello from controller context!?" - assert_status 200 - end - end -end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 945d2275c0..7f1c41787a 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -49,11 +49,11 @@ class RedirectController < ActionController::Base end def url_redirect_with_status - redirect_to("http://www.example.com", status: :moved_permanently, allow_other_host: true) + redirect_to("http://www.example.com", status: :moved_permanently) end def url_redirect_with_status_hash - redirect_to("http://www.example.com", status: 301, allow_other_host: true) + redirect_to("http://www.example.com", status: 301) end def relative_url_redirect_with_status @@ -68,10 +68,18 @@ class RedirectController < ActionController::Base redirect_back(fallback_location: "/things/stuff", status: 307) end + def redirect_back_with_status_and_fallback_location_to_another_host + redirect_back(fallback_location: "http://www.rubyonrails.org/", status: 307) + end + def safe_redirect_back_with_status redirect_back(fallback_location: "/things/stuff", status: 307, allow_other_host: false) end + def safe_redirect_back_with_status_and_fallback_location_to_another_host + redirect_back(fallback_location: "http://www.rubyonrails.org/", status: 307, allow_other_host: false) + end + def host_redirect redirect_to action: "other_host", only_path: false, host: "other.test.host" end @@ -81,27 +89,19 @@ class RedirectController < ActionController::Base end def redirect_to_url - redirect_to "http://www.rubyonrails.org/", allow_other_host: true - end - - def redirect_to_unsafe_url redirect_to "http://www.rubyonrails.org/" end - def redirect_to_relative_unsafe_url - redirect_to ".br" - end - def redirect_to_url_with_unescaped_query_string - redirect_to "http://example.com/query?status=new", allow_other_host: true + redirect_to "http://example.com/query?status=new" end def redirect_to_url_with_complex_scheme - redirect_to "x-test+scheme.complex:redirect", allow_other_host: true + redirect_to "x-test+scheme.complex:redirect" end def redirect_to_url_with_network_path_reference - redirect_to "//www.rubyonrails.org/", allow_other_host: true + redirect_to "//www.rubyonrails.org/" end def redirect_to_existing_record @@ -121,12 +121,12 @@ class RedirectController < ActionController::Base end def redirect_to_with_block - redirect_to proc { "http://www.rubyonrails.org/" }, allow_other_host: true + redirect_to proc { "http://www.rubyonrails.org/" } end def redirect_to_with_block_and_assigns @url = "http://www.rubyonrails.org/" - redirect_to proc { @url }, allow_other_host: true + redirect_to proc { @url } end def redirect_to_with_block_and_options @@ -253,28 +253,6 @@ class RedirectTest < ActionController::TestCase assert_redirected_to "http://www.rubyonrails.org/" end - def test_redirect_to_unsafe_url - error = assert_raises(ArgumentError) do - get :redirect_to_unsafe_url - end - assert_equal <<~MSG.squish, error.message - Unsafe redirect \"http://www.rubyonrails.org/\", - use :fallback_location to specify a fallback or - :allow_other_host to redirect anyway. - MSG - end - - def test_redirect_to_relative_unsafe_url - error = assert_raises(ArgumentError) do - get :redirect_to_relative_unsafe_url - end - assert_equal <<~MSG.squish, error.message - Unsafe redirect \"http://test.host.br\", - use :fallback_location to specify a fallback or - :allow_other_host to redirect anyway. - MSG - end - def test_redirect_to_url_with_unescaped_query_string get :redirect_to_url_with_unescaped_query_string assert_response :redirect @@ -310,6 +288,13 @@ class RedirectTest < ActionController::TestCase assert_equal "http://test.host/things/stuff", redirect_to_url end + def test_redirect_back_with_no_referer_redirects_to_another_host + get :redirect_back_with_status_and_fallback_location_to_another_host + + assert_response 307 + assert_equal "http://www.rubyonrails.org/", redirect_to_url + end + def test_safe_redirect_back_from_other_host @request.env["HTTP_REFERER"] = "http://another.host/coming/from" get :safe_redirect_back_with_status @@ -327,6 +312,20 @@ class RedirectTest < ActionController::TestCase assert_equal referer, redirect_to_url end + def test_safe_redirect_back_with_no_referer + get :safe_redirect_back_with_status + + assert_response 307 + assert_equal "http://test.host/things/stuff", redirect_to_url + end + + def test_safe_redirect_back_with_no_referer_redirects_to_another_host + get :safe_redirect_back_with_status_and_fallback_location_to_another_host + + assert_response 307 + assert_equal "http://www.rubyonrails.org/", redirect_to_url + end + def test_redirect_to_record with_routing do |set| set.draw do diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index c326f276bf..6914fb66f9 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -8,7 +8,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest class Boomer attr_accessor :closed - def initialize(detailed = false) + def initialize(detailed = false) @detailed = detailed @closed = false end @@ -39,52 +39,56 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest def call(env) env["action_dispatch.show_detailed_exceptions"] = @detailed req = ActionDispatch::Request.new(env) + template = ActionView::Template.new(File.read(__FILE__), __FILE__, ActionView::Template::Handlers::Raw.new, {}) + case req.path - when %r{/pass} + when "/pass" [404, { "X-Cascade" => "pass" }, self] - when %r{/not_found} + when "/not_found" raise AbstractController::ActionNotFound - when %r{/runtime_error} + when "/runtime_error" raise RuntimeError - when %r{/method_not_allowed} + when "/method_not_allowed" raise ActionController::MethodNotAllowed - when %r{/intercepted_error} + when "/intercepted_error" raise InterceptedErrorInstance - when %r{/unknown_http_method} + when "/unknown_http_method" raise ActionController::UnknownHttpMethod - when %r{/not_implemented} + when "/not_implemented" raise ActionController::NotImplemented - when %r{/unprocessable_entity} + when "/unprocessable_entity" raise ActionController::InvalidAuthenticityToken - when %r{/not_found_original_exception} + when "/not_found_original_exception" begin raise AbstractController::ActionNotFound.new rescue - raise ActionView::Template::Error.new("template") + raise ActionView::Template::Error.new(template) + end + when "/cause_mapped_to_rescue_responses" + begin + raise ActionController::ParameterMissing, :missing_param_key + rescue + raise NameError.new("uninitialized constant Userr") end - when %r{/missing_template} + when "/missing_template" raise ActionView::MissingTemplate.new(%w(foo), "foo/index", %w(foo), false, "mailer") - when %r{/bad_request} + when "/bad_request" raise ActionController::BadRequest - when %r{/missing_keys} + when "/missing_keys" raise ActionController::UrlGenerationError, "No route matches" - when %r{/parameter_missing} + when "/parameter_missing" raise ActionController::ParameterMissing, :missing_param_key - when %r{/original_syntax_error} + when "/original_syntax_error" eval "broke_syntax =" # `eval` need for raise native SyntaxError at runtime - when %r{/syntax_error_into_view} + when "/syntax_error_into_view" begin eval "broke_syntax =" rescue Exception - template = ActionView::Template.new(File.read(__FILE__), - __FILE__, - ActionView::Template::Handlers::Raw.new, - {}) raise ActionView::Template::Error.new(template) end - when %r{/framework_raises} + when "/framework_raises" method_that_raises - when %r{/nested_exceptions} + when "/nested_exceptions" raise_nested_exceptions else raise "puke!" @@ -313,12 +317,22 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(""foo"=>"[FILTERED]"", body) end - test "show registered original exception for wrapped exceptions" do + test "show registered original exception if the last exception is TemplateError" do @app = DevelopmentApp get "/not_found_original_exception", headers: { "action_dispatch.show_exceptions" => true } assert_response 404 - assert_match(/AbstractController::ActionNotFound/, body) + assert_match %r{AbstractController::ActionNotFound}, body + assert_match %r{Showing <i>.*test/dispatch/debug_exceptions_test.rb</i>}, body + end + + test "show the last exception and cause even when the cause is mapped to resque_responses" do + @app = DevelopmentApp + + get "/cause_mapped_to_rescue_responses", headers: { "action_dispatch.show_exceptions" => true } + assert_response 500 + assert_match %r{ActionController::ParameterMissing}, body + assert_match %r{NameError}, body end test "named urls missing keys raise 500 level error" do @@ -480,6 +494,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_select "#Application-Trace-0" do assert_select "code", /syntax error, unexpected/ end + assert_match %r{Showing <i>.*test/dispatch/debug_exceptions_test.rb</i>}, body end test "debug exceptions app shows user code that caused the error in source view" do diff --git a/actionpack/test/dispatch/live_response_test.rb b/actionpack/test/dispatch/live_response_test.rb index a9a56f205f..b673fd3805 100644 --- a/actionpack/test/dispatch/live_response_test.rb +++ b/actionpack/test/dispatch/live_response_test.rb @@ -62,7 +62,7 @@ module ActionController assert_nil @response.headers["Content-Length"] end - def test_headers_cannot_be_written_after_webserver_reads + def test_headers_cannot_be_written_after_web_server_reads @response.stream.write "omg" latch = Concurrent::CountDownLatch.new diff --git a/actionpack/test/dispatch/system_testing/driver_test.rb b/actionpack/test/dispatch/system_testing/driver_test.rb index a824ee0c84..0d08f17af3 100644 --- a/actionpack/test/dispatch/system_testing/driver_test.rb +++ b/actionpack/test/dispatch/system_testing/driver_test.rb @@ -2,6 +2,7 @@ require "abstract_unit" require "action_dispatch/system_testing/driver" +require "selenium/webdriver" class DriverTest < ActiveSupport::TestCase test "initializing the driver" do @@ -22,6 +23,7 @@ class DriverTest < ActiveSupport::TestCase driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :headless_chrome, screen_size: [1400, 1400], options: { url: "http://example.com/wd/hub" }) assert_equal :selenium, driver.instance_variable_get(:@name) assert_equal :headless_chrome, driver.instance_variable_get(:@browser).name + assert_instance_of Selenium::WebDriver::Chrome::Options, driver.instance_variable_get(:@browser).options assert_equal [1400, 1400], driver.instance_variable_get(:@screen_size) assert_equal ({ url: "http://example.com/wd/hub" }), driver.instance_variable_get(:@options) end @@ -30,6 +32,7 @@ class DriverTest < ActiveSupport::TestCase driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :headless_firefox, screen_size: [1400, 1400], options: { url: "http://example.com/wd/hub" }) assert_equal :selenium, driver.instance_variable_get(:@name) assert_equal :headless_firefox, driver.instance_variable_get(:@browser).name + assert_instance_of Selenium::WebDriver::Firefox::Options, driver.instance_variable_get(:@browser).options assert_equal [1400, 1400], driver.instance_variable_get(:@screen_size) assert_equal ({ url: "http://example.com/wd/hub" }), driver.instance_variable_get(:@options) end @@ -51,4 +54,70 @@ class DriverTest < ActiveSupport::TestCase test "registerable? returns false if driver is rack_test" do assert_not ActionDispatch::SystemTesting::Driver.new(:rack_test).send(:registerable?) end + + test "define extra capabilities using chrome" do + driver_option = nil + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :chrome) do |option| + option.add_argument("start-maximized") + option.add_emulation(device_name: "iphone 6") + option.add_preference(:detach, true) + + driver_option = option + end + driver.use + + expected = { args: ["start-maximized"], mobileEmulation: { deviceName: "iphone 6" }, prefs: { detach: true } } + assert_equal expected, driver_option.as_json + end + + test "define extra capabilities using headless_chrome" do + driver_option = nil + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :headless_chrome) do |option| + option.add_argument("start-maximized") + option.add_emulation(device_name: "iphone 6") + option.add_preference(:detach, true) + + driver_option = option + end + driver.use + + expected = { args: ["start-maximized"], mobileEmulation: { deviceName: "iphone 6" }, prefs: { detach: true } } + assert_equal expected, driver_option.as_json + end + + test "define extra capabilities using firefox" do + driver_option = nil + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :firefox) do |option| + option.add_preference("browser.startup.homepage", "http://www.seleniumhq.com/") + option.add_argument("--host=127.0.0.1") + + driver_option = option + end + driver.use + + expected = { "moz:firefoxOptions" => { args: ["--host=127.0.0.1"], prefs: { "browser.startup.homepage" => "http://www.seleniumhq.com/" } } } + assert_equal expected, driver_option.as_json + end + + test "define extra capabilities using headless_firefox" do + driver_option = nil + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :headless_firefox) do |option| + option.add_preference("browser.startup.homepage", "http://www.seleniumhq.com/") + option.add_argument("--host=127.0.0.1") + + driver_option = option + end + driver.use + + expected = { "moz:firefoxOptions" => { args: ["--host=127.0.0.1"], prefs: { "browser.startup.homepage" => "http://www.seleniumhq.com/" } } } + assert_equal expected, driver_option.as_json + end + + test "does not define extra capabilities" do + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :firefox) + + assert_nothing_raised do + driver.use + end + end end diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb index 097ef8af29..b756b91379 100644 --- a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb +++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb @@ -3,6 +3,7 @@ require "abstract_unit" require "action_dispatch/system_testing/test_helpers/screenshot_helper" require "capybara/dsl" +require "selenium/webdriver" class ScreenshotHelperTest < ActiveSupport::TestCase test "image path is saved in tmp directory" do diff --git a/actionpack/test/dispatch/system_testing/system_test_case_test.rb b/actionpack/test/dispatch/system_testing/system_test_case_test.rb index b078a5abc5..847b09dcfe 100644 --- a/actionpack/test/dispatch/system_testing/system_test_case_test.rb +++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "abstract_unit" +require "selenium/webdriver" class SetDriverToRackTestTest < DrivenByRackTest test "uses rack_test" do diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 1f4e14aef6..f8d89def6a 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -284,7 +284,7 @@ module ActionDispatch def test_generate_missing_keys_no_matches_different_format_keys get "/:controller/:action/:name", to: "foo#bar" - primarty_parameters = { + primary_parameters = { id: 1, controller: "tasks", action: "show", @@ -297,9 +297,9 @@ module ActionDispatch missing_parameters = { missing_key => "task_1" } - request_parameters = primarty_parameters.merge(redirection_parameters).merge(missing_parameters) + request_parameters = primary_parameters.merge(redirection_parameters).merge(missing_parameters) - message = "No route matches #{Hash[request_parameters.sort_by { |k, v|k.to_s }].inspect}, missing required keys: #{[missing_key.to_sym].inspect}" + message = "No route matches #{Hash[request_parameters.sort_by { |k, _|k.to_s }].inspect}, missing required keys: #{[missing_key.to_sym].inspect}" error = assert_raises(ActionController::UrlGenerationError) do @formatter.generate( diff --git a/actiontext/app/helpers/action_text/tag_helper.rb b/actiontext/app/helpers/action_text/tag_helper.rb index 837b2264b1..8434f2c611 100644 --- a/actiontext/app/helpers/action_text/tag_helper.rb +++ b/actiontext/app/helpers/action_text/tag_helper.rb @@ -4,7 +4,7 @@ module ActionText module TagHelper cattr_accessor(:id, instance_accessor: false) { 0 } - # Returns a `trix-editor` tag that instantiates the Trix JavaScript editor as well as a hidden field + # Returns a +trix-editor+ tag that instantiates the Trix JavaScript editor as well as a hidden field # that Trix will write to on changes, so the content will be sent on form submissions. # # ==== Options @@ -50,7 +50,7 @@ module ActionView::Helpers end module FormHelper - # Returns a `trix-editor` tag that instantiates the Trix JavaScript editor as well as a hidden field + # Returns a +trix-editor+ tag that instantiates the Trix JavaScript editor as well as a hidden field # that Trix will write to on changes, so the content will be sent on form submissions. # # ==== Options diff --git a/actiontext/app/models/action_text/rich_text.rb b/actiontext/app/models/action_text/rich_text.rb index 705dd30983..19fa3e030e 100644 --- a/actiontext/app/models/action_text/rich_text.rb +++ b/actiontext/app/models/action_text/rich_text.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true module ActionText - # The RichText record holds the content produced by the Trix editor in a serialized `body` attribute. + # The RichText record holds the content produced by the Trix editor in a serialized +body+ attribute. # It also holds all the references to the embedded files, which are stored using Active Storage. # This record is then associated with the Active Record model the application desires to have - # rich text content using the `has_rich_text` class method. + # rich text content using the +has_rich_text+ class method. class RichText < ActiveRecord::Base self.table_name = "action_text_rich_texts" @@ -15,7 +15,7 @@ module ActionText has_many_attached :embeds before_save do - self.embeds = body.attachments.map(&:attachable) if body.present? + self.embeds = body.attachables.grep(ActiveStorage::Blob) if body.present? end def to_plain_text diff --git a/actiontext/app/views/active_storage/blobs/_blob.html.erb b/actiontext/app/views/active_storage/blobs/_blob.html.erb index 049f57e804..49ba357dd1 100644 --- a/actiontext/app/views/active_storage/blobs/_blob.html.erb +++ b/actiontext/app/views/active_storage/blobs/_blob.html.erb @@ -1,6 +1,6 @@ <figure class="attachment attachment--<%= blob.representable? ? "preview" : "file" %> attachment--<%= blob.filename.extension %>"> <% if blob.representable? %> - <%= image_tag blob.representation(resize_to_fit: local_assigns[:in_gallery] ? [ 800, 600 ] : [ 1024, 768 ]) %> + <%= image_tag blob.representation(resize_to_limit: local_assigns[:in_gallery] ? [ 800, 600 ] : [ 1024, 768 ]) %> <% end %> <figcaption class="attachment__caption"> diff --git a/actiontext/db/migrate/201805281641_create_action_text_tables.rb b/actiontext/db/migrate/20180528164100_create_action_text_tables.rb index 74c7a0ecb9..e7c66ea6ae 100644 --- a/actiontext/db/migrate/201805281641_create_action_text_tables.rb +++ b/actiontext/db/migrate/20180528164100_create_action_text_tables.rb @@ -2,11 +2,10 @@ class CreateActionTextTables < ActiveRecord::Migration[6.0] def change create_table :action_text_rich_texts do |t| t.string :name, null: false - t.text :body, limit: 16777215 + t.text :body, size: :long t.references :record, null: false, polymorphic: true, index: false - t.datetime :created_at, null: false - t.datetime :updated_at, null: false + t.timestamps t.index [ :record_type, :record_id, :name ], name: "index_action_text_rich_texts_uniqueness", unique: true end diff --git a/actiontext/lib/templates/installer.rb b/actiontext/lib/templates/installer.rb index 990e41ca00..a8000eb9fc 100644 --- a/actiontext/lib/templates/installer.rb +++ b/actiontext/lib/templates/installer.rb @@ -26,7 +26,7 @@ if APPLICATION_PACK_PATH.exist? line = %[require("#{name}")] unless APPLICATION_PACK_PATH.read.include? line say "Adding #{name} to #{APPLICATION_PACK_PATH}" - append_to_file APPLICATION_PACK_PATH, "#{line}\n" + append_to_file APPLICATION_PACK_PATH, "\n#{line}" end end end diff --git a/actiontext/test/dummy/config/puma.rb b/actiontext/test/dummy/config/puma.rb index a5eccf816b..71ed69a895 100644 --- a/actiontext/test/dummy/config/puma.rb +++ b/actiontext/test/dummy/config/puma.rb @@ -16,7 +16,7 @@ port ENV.fetch("PORT") { 3000 } environment ENV.fetch("RAILS_ENV") { "development" } # Specifies the number of `workers` to boot in clustered mode. -# Workers are forked webserver processes. If using threads and workers together +# Workers are forked web server processes. If using threads and workers together # the concurrency of the application would be max `threads` * `workers`. # Workers do not work on JRuby or Windows (both of which do not support # processes). diff --git a/actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb b/actiontext/test/dummy/db/migrate/20180528164100_create_action_text_tables.rb index 74c7a0ecb9..e7c66ea6ae 100644 --- a/actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb +++ b/actiontext/test/dummy/db/migrate/20180528164100_create_action_text_tables.rb @@ -2,11 +2,10 @@ class CreateActionTextTables < ActiveRecord::Migration[6.0] def change create_table :action_text_rich_texts do |t| t.string :name, null: false - t.text :body, limit: 16777215 + t.text :body, size: :long t.references :record, null: false, polymorphic: true, index: false - t.datetime :created_at, null: false - t.datetime :updated_at, null: false + t.timestamps t.index [ :record_type, :record_id, :name ], name: "index_action_text_rich_texts_uniqueness", unique: true end diff --git a/actiontext/test/dummy/db/schema.rb b/actiontext/test/dummy/db/schema.rb index 7f8f4dff4e..5216c5fd33 100644 --- a/actiontext/test/dummy/db/schema.rb +++ b/actiontext/test/dummy/db/schema.rb @@ -14,11 +14,11 @@ ActiveRecord::Schema.define(version: 2018_10_03_185713) do create_table "action_text_rich_texts", force: :cascade do |t| t.string "name", null: false - t.text "body", limit: 16777215 + t.text "body" t.string "record_type", null: false t.integer "record_id", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false t.index ["record_type", "record_id", "name"], name: "index_action_text_rich_texts_uniqueness", unique: true end @@ -45,14 +45,14 @@ ActiveRecord::Schema.define(version: 2018_10_03_185713) do create_table "messages", force: :cascade do |t| t.string "subject" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false end create_table "people", force: :cascade do |t| t.string "name" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false end end diff --git a/actiontext/test/unit/model_test.rb b/actiontext/test/unit/model_test.rb index 122a20700b..d56363adc0 100644 --- a/actiontext/test/unit/model_test.rb +++ b/actiontext/test/unit/model_test.rb @@ -35,6 +35,15 @@ class ActionText::ModelTest < ActiveSupport::TestCase assert_equal "racecar.jpg", message.content.embeds.first.filename.to_s end + test "embed extraction only extracts file attachments" do + remote_image_html = '<action-text-attachment content-type="image" url="http://example.com/cat.jpg"></action-text-attachment>' + blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg") + content = ActionText::Content.new(remote_image_html).append_attachables(blob) + message = Message.create!(subject: "Greetings", content: content) + assert_equal [ActionText::Attachables::RemoteImage, ActiveStorage::Blob], message.content.body.attachables.map(&:class) + assert_equal [ActiveStorage::Attachment], message.content.embeds.map(&:class) + end + test "saving content" do message = Message.create!(subject: "Greetings", content: "<h1>Hello world</h1>") assert_equal "Hello world", message.content.to_plain_text diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 9e47d81faf..e2cd9f7558 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,5 +1,11 @@ ## Rails 6.0.0.beta1 (January 18, 2019) ## +* [Rename npm package](https://github.com/rails/rails/pull/34905) from + [`rails-ujs`](https://www.npmjs.com/package/rails-ujs) to + [`@rails/ujs`](https://www.npmjs.com/package/@rails/ujs). + + *Javan Makhmali* + * Remove deprecated `image_alt` helper. *Rafael Mendonça França* @@ -10,7 +16,8 @@ *Genadi Samokovarov* * Fix UJS permanently showing disabled text in a[data-remote][data-disable-with] elements within forms. - Fixes #33889 + + Fixes #33889. *Wolfgang Hobmaier* @@ -22,7 +29,7 @@ <%= link_to 'Remote', remote_path, class: 'remote', remote: true, data: { type: :json } %> ``` - Fixes #34541 + Fixes #34541. *Wolfgang Hobmaier* @@ -39,7 +46,7 @@ Calling `word_wrap` should not trim the indents on the first and last lines. - Fixes #34487 + Fixes #34487. *Lyle Mullican* diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index d41fe2a608..fe9f3f9503 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -3,6 +3,7 @@ require "active_support/core_ext/module/attr_internal" require "active_support/core_ext/module/attribute_accessors" require "active_support/ordered_options" +require "active_support/deprecation" require "action_view/log_subscriber" require "action_view/helpers" require "action_view/context" @@ -10,6 +11,10 @@ require "action_view/template" require "action_view/lookup_context" module ActionView #:nodoc: + module CompiledTemplates #:nodoc: + # holds compiled template code + end + # = Action View Base # # Action View templates can be written in several ways. @@ -141,6 +146,8 @@ module ActionView #:nodoc: class Base include Helpers, ::ERB::Util, Context + include CompiledTemplates + # Specify the proc used to decorate input tags that refer to attributes with errors. cattr_accessor :field_error_proc, default: Proc.new { |html_tag, instance| "<div class=\"field_with_errors\">#{html_tag}</div>".html_safe } @@ -181,7 +188,7 @@ module ActionView #:nodoc: end end - attr_accessor :view_renderer + attr_reader :view_renderer attr_internal :config, :assigns delegate :lookup_context, to: :view_renderer @@ -191,17 +198,49 @@ module ActionView #:nodoc: @_assigns = new_assigns.each { |key, value| instance_variable_set("@#{key}", value) } end - def initialize(context = nil, assigns = {}, controller = nil, formats = nil) #:nodoc: + # :stopdoc: + + def self.build_renderer(context, controller, formats) + lookup_context = context.is_a?(ActionView::LookupContext) ? + context : ActionView::LookupContext.new(context) + lookup_context.formats = formats if formats + lookup_context.prefixes = controller._prefixes if controller + ActionView::Renderer.new(lookup_context) + end + + def self.empty + with_view_paths([]) + end + + def self.with_view_paths(view_paths, assigns = {}, controller = nil) + with_context ActionView::LookupContext.new(view_paths), assigns, controller + end + + def self.with_context(context, assigns = {}, controller = nil) + new ActionView::Renderer.new(context), assigns, controller + end + + NULL = Object.new + + # :startdoc: + + def initialize(renderer, assigns = {}, controller = nil, formats = NULL) #:nodoc: @_config = ActiveSupport::InheritableOptions.new - if context.is_a?(ActionView::Renderer) - @view_renderer = context + unless formats == NULL + ActiveSupport::Deprecation.warn <<~eowarn + Passing formats to ActionView::Base.new is deprecated + eowarn + end + + if renderer.is_a?(ActionView::Renderer) + @view_renderer = renderer else - lookup_context = context.is_a?(ActionView::LookupContext) ? - context : ActionView::LookupContext.new(context) - lookup_context.formats = formats if formats - lookup_context.prefixes = controller._prefixes if controller - @view_renderer = ActionView::Renderer.new(lookup_context) + ActiveSupport::Deprecation.warn <<~eowarn + ActionView::Base instances should be constructed with a view renderer, + assigments, and a controller. + eowarn + @view_renderer = self.class.build_renderer(renderer, controller, formats) end @cache_hit = {} @@ -210,6 +249,18 @@ module ActionView #:nodoc: _prepare_context end + def run(method, locals, buffer, &block) + _old_output_buffer, _old_virtual_path = @output_buffer, @virtual_path + @output_buffer = buffer + send(method, locals, buffer, &block) + ensure + @output_buffer, @virtual_path = _old_output_buffer, _old_virtual_path + end + + def compiled_method_container + CompiledTemplates + end + ActiveSupport.run_load_hooks(:action_view, self) end end diff --git a/actionview/lib/action_view/context.rb b/actionview/lib/action_view/context.rb index 3c605c3ee3..2b22c30a3a 100644 --- a/actionview/lib/action_view/context.rb +++ b/actionview/lib/action_view/context.rb @@ -1,10 +1,6 @@ # frozen_string_literal: true module ActionView - module CompiledTemplates #:nodoc: - # holds compiled template code - end - # = Action View Context # # Action View contexts are supplied to Action Controller to render a template. @@ -16,7 +12,6 @@ module ActionView # object that includes this module (although you can call _prepare_context # defined below). module Context - include CompiledTemplates attr_accessor :output_buffer, :view_flow # Prepares the context by setting the appropriate instance variables. diff --git a/actionview/lib/action_view/helpers/asset_tag_helper.rb b/actionview/lib/action_view/helpers/asset_tag_helper.rb index c186cb8422..59d70a1dc4 100644 --- a/actionview/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionview/lib/action_view/helpers/asset_tag_helper.rb @@ -329,14 +329,14 @@ module ActionView # image_tag("pic.jpg", srcset: [["pic_1024.jpg", "1024w"], ["pic_1980.jpg", "1980w"]], sizes: "100vw") # # => <img src="/assets/pic.jpg" srcset="/assets/pic_1024.jpg 1024w, /assets/pic_1980.jpg 1980w" sizes="100vw"> # - # Active Storage (images that are uploaded by the users of your app): + # Active Storage blobs (images that are uploaded by the users of your app): # # image_tag(user.avatar) # # => <img src="/rails/active_storage/blobs/.../tiger.jpg" /> - # image_tag(user.avatar.variant(resize_to_fit: [100, 100])) - # # => <img src="/rails/active_storage/variants/.../tiger.jpg" /> - # image_tag(user.avatar.variant(resize_to_fit: [100, 100]), size: '100') - # # => <img width="100" height="100" src="/rails/active_storage/variants/.../tiger.jpg" /> + # image_tag(user.avatar.variant(resize_to_limit: [100, 100])) + # # => <img src="/rails/active_storage/representations/.../tiger.jpg" /> + # image_tag(user.avatar.variant(resize_to_limit: [100, 100]), size: '100') + # # => <img width="100" height="100" src="/rails/active_storage/representations/.../tiger.jpg" /> def image_tag(source, options = {}) options = options.symbolize_keys check_for_image_tag_errors(options) diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 554d223c0e..c2f6439e26 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -3,6 +3,7 @@ require "concurrent/map" require "active_support/core_ext/module/remove_method" require "active_support/core_ext/module/attribute_accessors" +require "active_support/deprecation" require "action_view/template/resolver" module ActionView @@ -106,12 +107,6 @@ module ActionView module ViewPaths attr_reader :view_paths, :html_fallback_for_js - # Whenever setting view paths, makes a copy so that we can manipulate them in - # instance objects as we wish. - def view_paths=(paths) - @view_paths = ActionView::PathSet.new(Array(paths)) - end - def find(name, prefixes = [], partial = false, keys = [], options = {}) @view_paths.find(*args_for_lookup(name, prefixes, partial, keys, options)) end @@ -138,19 +133,34 @@ module ActionView # Adds fallbacks to the view paths. Useful in cases when you are rendering # a :file. def with_fallbacks - added_resolvers = 0 - self.class.fallbacks.each do |resolver| - next if view_paths.include?(resolver) - view_paths.push(resolver) - added_resolvers += 1 + view_paths = build_view_paths((@view_paths.paths + self.class.fallbacks).uniq) + + if block_given? + ActiveSupport::Deprecation.warn <<~eowarn + Calling `with_fallbacks` with a block is deprecated. Call methods on + the lookup context returned by `with_fallbacks` instead. + eowarn + + begin + _view_paths = @view_paths + @view_paths = view_paths + yield + ensure + @view_paths = _view_paths + end + else + ActionView::LookupContext.new(view_paths, @details, @prefixes) end - yield - ensure - added_resolvers.times { view_paths.pop } end private + # Whenever setting view paths, makes a copy so that we can manipulate them in + # instance objects as we wish. + def build_view_paths(paths) + ActionView::PathSet.new(Array(paths)) + end + def args_for_lookup(name, prefixes, partial, keys, details_options) name, prefixes = normalize_name(name, prefixes) details, details_key = detail_args_for(details_options) @@ -226,7 +236,7 @@ module ActionView @rendered_format = nil @details = initialize_details({}, details) - self.view_paths = view_paths + @view_paths = build_view_paths(view_paths) end def digest_cache diff --git a/actionview/lib/action_view/renderer/abstract_renderer.rb b/actionview/lib/action_view/renderer/abstract_renderer.rb index 20b2523cac..ae366ce54a 100644 --- a/actionview/lib/action_view/renderer/abstract_renderer.rb +++ b/actionview/lib/action_view/renderer/abstract_renderer.rb @@ -17,7 +17,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?, :any_templates?, :with_fallbacks, :with_layout_format, :formats, to: :@lookup_context + delegate :template_exists?, :any_templates?, :formats, to: :@lookup_context def initialize(lookup_context) @lookup_context = lookup_context @@ -38,8 +38,6 @@ module ActionView end def instrument(name, **options) # :doc: - options[:identifier] ||= (@template && @template.identifier) || @path - ActiveSupport::Notifications.instrument("render_#{name}.action_view", options) do |payload| yield payload end diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index cb850d75ee..478400e016 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -296,42 +296,43 @@ module ActionView def render(context, options, block) setup(context, options, block) - @template = find_partial + template = find_partial @lookup_context.rendered_format ||= begin - if @template && @template.formats.present? - @template.formats.first + if template && template.formats.first + template.formats.first else formats.first end end if @collection - render_collection + render_collection(context, template) else - render_partial + render_partial(context, template) end end private - def render_collection - instrument(:collection, count: @collection.size) do |payload| + def render_collection(view, template) + identifier = (template && template.identifier) || @path + instrument(:collection, identifier: identifier, count: @collection.size) do |payload| return nil if @collection.blank? if @options.key?(:spacer_template) - spacer = find_template(@options[:spacer_template], @locals.keys).render(@view, @locals) + spacer = find_template(@options[:spacer_template], @locals.keys).render(view, @locals) end - cache_collection_render(payload) do - @template ? collection_with_template : collection_without_template + cache_collection_render(payload, view, template) do + template ? collection_with_template(view, template) : collection_without_template(view) end.join(spacer).html_safe end end - def render_partial - instrument(:partial) do |payload| - view, locals, block = @view, @locals, @block + def render_partial(view, template) + instrument(:partial, identifier: template.identifier) do |payload| + locals, block = @locals, @block object, as = @object, @variable if !block && (layout = @options[:layout]) @@ -341,12 +342,12 @@ module ActionView object = locals[as] if object.nil? # Respect object when object is false locals[as] = object if @has_object - content = @template.render(view, locals) do |*name| + content = template.render(view, locals) do |*name| view._layout_for(*name, &block) end content = layout.render(view, locals) { content } if layout - payload[:cache_hit] = view.view_renderer.cache_hits[@template.virtual_path] + payload[:cache_hit] = view.view_renderer.cache_hits[template.virtual_path] content end end @@ -359,11 +360,10 @@ module ActionView # set to that string. Otherwise, the +options[:partial]+ object must # respond to +to_partial_path+ in order to setup the path. def setup(context, options, block) - @view = context @options = options @block = block - @locals = options[:locals] ? options[:locals].symbolize_keys : {} + @locals = options[:locals] || {} @details = extract_details(options) prepend_formats(options[:formats]) @@ -381,10 +381,10 @@ module ActionView @collection = collection_from_object || collection_from_options if @collection - paths = @collection_data = @collection.map { |o| partial_path(o) } + paths = @collection_data = @collection.map { |o| partial_path(o, context) } @path = paths.uniq.one? ? paths.first : nil else - @path = partial_path + @path = partial_path(@object, context) end end @@ -423,8 +423,8 @@ module ActionView @lookup_context.find_template(path, prefixes, true, locals, @details) end - def collection_with_template - view, locals, template = @view, @locals, @template + def collection_with_template(view, template) + locals = @locals as, counter, iteration = @variable, @variable_counter, @variable_iteration if layout = @options[:layout] @@ -445,8 +445,8 @@ module ActionView end end - def collection_without_template - view, locals, collection_data = @view, @locals, @collection_data + def collection_without_template(view) + locals, collection_data = @locals, @collection_data cache = {} keys = @locals.keys @@ -474,7 +474,7 @@ module ActionView # # If +prefix_partial_path_with_controller_namespace+ is true, then this # method will prefix the partial paths with a namespace. - def partial_path(object = @object) + def partial_path(object, view) object = object.to_model if object.respond_to?(:to_model) path = if object.respond_to?(:to_partial_path) @@ -483,7 +483,7 @@ module ActionView raise ArgumentError.new("'#{object.inspect}' is not an ActiveModel-compatible object. It must implement :to_partial_path.") end - if @view.prefix_partial_path_with_controller_namespace + if view.prefix_partial_path_with_controller_namespace prefixed_partial_names[path] ||= merge_prefix_into_object_path(@context_prefix, path.dup) else path diff --git a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb index 5aa6f77902..388f9e5e56 100644 --- a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb +++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb @@ -11,13 +11,13 @@ module ActionView end private - def cache_collection_render(instrumentation_payload) + def cache_collection_render(instrumentation_payload, view, template) return yield unless @options[:cached] # Result is a hash with the key represents the # key used for cache lookup and the value is the item # on which the partial is being rendered - keyed_collection = collection_by_cache_keys + keyed_collection = collection_by_cache_keys(view, template) # Pull all partials from cache # Result is a hash, key matches the entry in @@ -51,23 +51,21 @@ module ActionView @options[:cached].respond_to?(:call) end - def collection_by_cache_keys + def collection_by_cache_keys(view, template) seed = callable_cache_key? ? @options[:cached] : ->(i) { i } + digest_path = view.digest_path_from_virtual(template.virtual_path) + @collection.each_with_object({}) do |item, hash| - hash[expanded_cache_key(seed.call(item))] = item + hash[expanded_cache_key(seed.call(item), view, template, digest_path)] = item end end - def expanded_cache_key(key) - key = @view.combined_fragment_cache_key(@view.cache_fragment_name(key, virtual_path: @template.virtual_path, digest_path: digest_path)) + def expanded_cache_key(key, view, template, digest_path) + key = view.combined_fragment_cache_key(view.cache_fragment_name(key, virtual_path: template.virtual_path, digest_path: digest_path)) key.frozen? ? key.dup : key # #read_multi & #write may require mutability, Dalli 2.6.0. end - def digest_path - @digest_path ||= @view.digest_path_from_virtual(@template.virtual_path) - end - # `order_by` is an enumerable object containing keys of the cache, # all keys are passed in whether found already or not. # diff --git a/actionview/lib/action_view/renderer/streaming_template_renderer.rb b/actionview/lib/action_view/renderer/streaming_template_renderer.rb index bb9db21e32..f414620923 100644 --- a/actionview/lib/action_view/renderer/streaming_template_renderer.rb +++ b/actionview/lib/action_view/renderer/streaming_template_renderer.rb @@ -43,14 +43,14 @@ module ActionView # For streaming, instead of rendering a given a template, we return a Body # object that responds to each. This object is initialized with a block # that knows how to render the template. - def render_template(template, layout_name = nil, locals = {}) #:nodoc: + def render_template(view, template, layout_name = nil, locals = {}) #:nodoc: return [super] unless layout_name && template.supports_streaming? locals ||= {} layout = layout_name && find_layout(layout_name, locals.keys, [formats.first]) Body.new do |buffer| - delayed_render(buffer, template, layout, @view, locals) + delayed_render(buffer, template, layout, view, locals) end end diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index ce8908924a..c36baeffcd 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -5,7 +5,6 @@ require "active_support/core_ext/object/try" module ActionView class TemplateRenderer < AbstractRenderer #:nodoc: def render(context, options) - @view = context @details = extract_details(options) template = determine_template(options) @@ -13,7 +12,7 @@ module ActionView @lookup_context.rendered_format ||= (template.formats.first || formats.first) - render_template(template, options[:layout], options[:locals]) + render_template(context, template, options[:layout], options[:locals] || {}) end private @@ -29,7 +28,7 @@ module ActionView elsif options.key?(:html) Template::HTML.new(options[:html], formats.first) elsif options.key?(:file) - with_fallbacks { find_file(options[:file], nil, false, keys, @details) } + @lookup_context.with_fallbacks.find_file(options[:file], nil, false, keys, @details) elsif options.key?(:inline) handler = Template.handler_for_extension(options[:type] || "erb") Template.new(options[:inline], "inline template", handler, locals: keys) @@ -37,7 +36,7 @@ module ActionView if options[:template].respond_to?(:render) options[:template] else - find_template(options[:template], options[:prefixes], false, keys, @details) + @lookup_context.find_template(options[:template], options[:prefixes], false, keys, @details) end else raise ArgumentError, "You invoked render but did not give any of :partial, :template, :inline, :file, :plain, :html or :body option." @@ -46,22 +45,19 @@ module ActionView # Renders the given template. A string representing the layout can be # supplied as well. - def render_template(template, layout_name = nil, locals = nil) - view, locals = @view, locals || {} - - render_with_layout(layout_name, locals) do |layout| + def render_template(view, template, layout_name, locals) + render_with_layout(view, layout_name, locals) do |layout| instrument(:template, identifier: template.identifier, layout: layout.try(:virtual_path)) do template.render(view, locals) { |*name| view._layout_for(*name) } end end end - def render_with_layout(path, locals) + def render_with_layout(view, path, locals) layout = path && find_layout(path, locals.keys, [formats.first]) content = yield(layout) if layout - view = @view view.view_flow.set(:layout, content) layout.render(view, locals) { |*name| view._layout_for(*name) } else @@ -84,9 +80,9 @@ module ActionView when String begin if layout.start_with?("/") - with_fallbacks { find_template(layout, nil, false, [], details) } + @lookup_context.with_fallbacks.find_template(layout, nil, false, [], details) else - find_template(layout, nil, false, [], details) + @lookup_context.find_template(layout, nil, false, [], details) end rescue ActionView::MissingTemplate all_details = @details.merge(formats: @lookup_context.default_formats) diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index cb4327cf16..205665a8c6 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -55,10 +55,8 @@ module ActionView end end - attr_internal_writer :view_context_class - def view_context_class - @_view_context_class ||= self.class.view_context_class + self.class.view_context_class end # An instance of a view class. The default view class is ActionView::Base. diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 070d82cf17..8ce8053011 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -155,10 +155,10 @@ module ActionView # This method is instrumented as "!render_template.action_view". Notice that # we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. - def render(view, locals, buffer = nil, &block) + def render(view, locals, buffer = ActionView::OutputBuffer.new, &block) instrument_render_template do compile!(view) - view.send(method_name, locals, buffer, &block) + view.run(method_name, locals, buffer, &block) end rescue => e handle_render_error(view, e) @@ -264,11 +264,7 @@ module ActionView # re-compilation return if @compiled - if view.is_a?(ActionView::CompiledTemplates) - mod = ActionView::CompiledTemplates - else - mod = view.singleton_class - end + mod = view.compiled_method_container instrument("!compile_template") do compile(mod) @@ -301,9 +297,7 @@ module ActionView # encoding of the code source = +<<-end_src def #{method_name}(local_assigns, output_buffer) - _old_virtual_path, @virtual_path = @virtual_path, #{@virtual_path.inspect};_old_output_buffer = @output_buffer;#{locals_code};#{code} - ensure - @virtual_path, @output_buffer = _old_virtual_path, _old_output_buffer + @virtual_path = #{@virtual_path.inspect};#{locals_code};#{code} end end_src diff --git a/actionview/lib/action_view/template/handlers/erb/erubi.rb b/actionview/lib/action_view/template/handlers/erb/erubi.rb index db75f028ed..15ca202024 100644 --- a/actionview/lib/action_view/template/handlers/erb/erubi.rb +++ b/actionview/lib/action_view/template/handlers/erb/erubi.rb @@ -13,7 +13,7 @@ module ActionView # Dup properties so that we don't modify argument properties = Hash[properties] - properties[:preamble] = "@output_buffer = output_buffer || ActionView::OutputBuffer.new;" + properties[:preamble] = "" properties[:postamble] = "@output_buffer.to_s" properties[:bufvar] = "@output_buffer" properties[:escapefunc] = "" @@ -22,8 +22,12 @@ module ActionView end def evaluate(action_view_erb_handler_context) - pr = eval("proc { #{@src} }", binding, @filename || "(erubi)") - action_view_erb_handler_context.instance_eval(&pr) + src = @src + view = Class.new(ActionView::Base) { + include action_view_erb_handler_context._routes.url_helpers + class_eval("define_method(:_template) { |local_assigns, output_buffer| #{src} }", @filename || "(erubi)", 0) + }.with_context(action_view_erb_handler_context) + view.run(:_template, {}, ActionView::OutputBuffer.new) end private diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index 1fad08a689..d6203b95c5 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -8,13 +8,15 @@ module ActionView #:nodoc: # useful for testing extensions that have no way of knowing what the file # system will look like at runtime. class FixtureResolver < PathResolver - attr_reader :hash - def initialize(hash = {}, pattern = nil) super(pattern) @hash = hash end + def data + @hash + end + def to_s @hash.keys.join(", ") end diff --git a/actionview/test/abstract_unit.rb b/actionview/test/abstract_unit.rb index f90626ad9e..56d617309c 100644 --- a/actionview/test/abstract_unit.rb +++ b/actionview/test/abstract_unit.rb @@ -48,7 +48,7 @@ module RenderERBUtils @view ||= begin path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH) view_paths = ActionView::PathSet.new([path]) - ActionView::Base.new(view_paths) + ActionView::Base.with_view_paths(view_paths) end end @@ -61,7 +61,7 @@ module RenderERBUtils ActionView::Template::Handlers::ERB, {}) - template.render(self, {}).strip + template.render(ActionView::Base.empty, {}).strip end end diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index 204903c60c..727d3fbc1a 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -485,8 +485,8 @@ class TestController < ActionController::Base render partial: "customer", locals: { customer: Customer.new("david") } end - def partial_with_string_locals - render partial: "customer", locals: { "customer" => Customer.new("david") } + def partial_with_hashlike_locals + render partial: "customer", locals: ActionController::Parameters.new(customer: Customer.new("david")) end def partial_with_form_builder @@ -691,7 +691,7 @@ class RenderTest < ActionController::TestCase get :partial_with_locals, to: "test#partial_with_locals" get :partial_with_nested_object, to: "test#partial_with_nested_object" get :partial_with_nested_object_shorthand, to: "test#partial_with_nested_object_shorthand" - get :partial_with_string_locals, to: "test#partial_with_string_locals" + get :partial_with_hashlike_locals, to: "test#partial_with_hashlike_locals" get :partials_list, to: "test#partials_list" get :render_action_hello_world, to: "test#render_action_hello_world" get :render_action_hello_world_as_string, to: "test#render_action_hello_world_as_string" @@ -1292,8 +1292,8 @@ class RenderTest < ActionController::TestCase assert_equal "Hello: david", @response.body end - def test_partial_with_string_locals - get :partial_with_string_locals + def test_partial_with_hashlike_locals + get :partial_with_hashlike_locals assert_equal "Hello: david", @response.body end diff --git a/actionview/test/actionpack/controller/view_paths_test.rb b/actionview/test/actionpack/controller/view_paths_test.rb index 7f3fe0fa08..c5238dd746 100644 --- a/actionview/test/actionpack/controller/view_paths_test.rb +++ b/actionview/test/actionpack/controller/view_paths_test.rb @@ -77,7 +77,7 @@ class ViewLoadPathsTest < ActionController::TestCase end def test_template_appends_view_path_correctly - @controller.instance_variable_set :@template, ActionView::Base.new(TestController.view_paths, {}, @controller) + @controller.instance_variable_set :@template, ActionView::Base.with_view_paths(TestController.view_paths, {}, @controller) class_view_paths = TestController.view_paths @controller.append_view_path "foo" @@ -89,7 +89,7 @@ class ViewLoadPathsTest < ActionController::TestCase end def test_template_prepends_view_path_correctly - @controller.instance_variable_set :@template, ActionView::Base.new(TestController.view_paths, {}, @controller) + @controller.instance_variable_set :@template, ActionView::Base.with_view_paths(TestController.view_paths, {}, @controller) class_view_paths = TestController.view_paths @controller.prepend_view_path "baz" diff --git a/actionview/test/activerecord/multifetch_cache_test.rb b/actionview/test/activerecord/multifetch_cache_test.rb index 12be069e69..229b4e56d0 100644 --- a/actionview/test/activerecord/multifetch_cache_test.rb +++ b/actionview/test/activerecord/multifetch_cache_test.rb @@ -19,7 +19,7 @@ class MultifetchCacheTest < ActiveRecordTestCase def combined_fragment_cache_key(key) [ :views, key ] end - end.new(view_paths, {}) + end.with_view_paths(view_paths, {}) end def test_only_preloading_for_records_that_miss_the_cache diff --git a/actionview/test/template/capture_helper_test.rb b/actionview/test/template/capture_helper_test.rb index e172497c88..45070674ad 100644 --- a/actionview/test/template/capture_helper_test.rb +++ b/actionview/test/template/capture_helper_test.rb @@ -5,7 +5,7 @@ require "abstract_unit" class CaptureHelperTest < ActionView::TestCase def setup super - @av = ActionView::Base.new + @av = ActionView::Base.empty @view_flow = ActionView::OutputFlow.new end diff --git a/actionview/test/template/compiled_templates_test.rb b/actionview/test/template/compiled_templates_test.rb index 3cd6448e38..ded4786e62 100644 --- a/actionview/test/template/compiled_templates_test.rb +++ b/actionview/test/template/compiled_templates_test.rb @@ -72,13 +72,13 @@ class CompiledTemplatesTest < ActiveSupport::TestCase def render_with_cache(*args) view_paths = ActionController::Base.view_paths - ActionView::Base.new(view_paths, {}).render(*args) + ActionView::Base.with_view_paths(view_paths, {}).render(*args) end def render_without_cache(*args) path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH) view_paths = ActionView::PathSet.new([path]) - ActionView::Base.new(view_paths, {}).render(*args) + ActionView::Base.with_view_paths(view_paths, {}).render(*args) end def modify_template(template, content) diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index 68e151f154..290f832794 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -5,10 +5,14 @@ require "abstract_controller/rendering" class LookupContextTest < ActiveSupport::TestCase def setup - @lookup_context = ActionView::LookupContext.new(FIXTURE_LOAD_PATH, {}) + @lookup_context = build_lookup_context(FIXTURE_LOAD_PATH, {}) ActionView::LookupContext::DetailsKey.clear end + def build_lookup_context(paths, details) + ActionView::LookupContext.new(paths, details) + end + def teardown I18n.locale = :en end @@ -118,19 +122,32 @@ class LookupContextTest < ActiveSupport::TestCase test "adds fallbacks to view paths when required" do assert_equal 1, @lookup_context.view_paths.size - @lookup_context.with_fallbacks do - assert_equal 3, @lookup_context.view_paths.size - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("") - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/") + assert_deprecated do + @lookup_context.with_fallbacks do + assert_equal 3, @lookup_context.view_paths.size + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("") + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/") + end end + + @lookup_context = @lookup_context.with_fallbacks + + assert_equal 3, @lookup_context.view_paths.size + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("") + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/") end test "add fallbacks just once in nested fallbacks calls" do - @lookup_context.with_fallbacks do + assert_deprecated do @lookup_context.with_fallbacks do - assert_equal 3, @lookup_context.view_paths.size + @lookup_context.with_fallbacks do + assert_equal 3, @lookup_context.view_paths.size + end end end + + @lookup_context = @lookup_context.with_fallbacks.with_fallbacks + assert_equal 3, @lookup_context.view_paths.size end test "generates a new details key for each details hash" do @@ -156,13 +173,13 @@ class LookupContextTest < ActiveSupport::TestCase end test "gives the key forward to the resolver, so it can be used as cache key" do - @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo") + @lookup_context = build_lookup_context(ActionView::FixtureResolver.new("test/_foo.erb" => "Foo"), {}) template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source # Now we are going to change the template, but it won't change the returned template # since we will hit the cache. - @lookup_context.view_paths.first.hash["test/_foo.erb"] = "Bar" + @lookup_context.view_paths.first.data["test/_foo.erb"] = "Bar" template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source @@ -185,7 +202,7 @@ class LookupContextTest < ActiveSupport::TestCase end test "can disable the cache on demand" do - @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo") + @lookup_context = build_lookup_context(ActionView::FixtureResolver.new("test/_foo.erb" => "Foo"), {}) old_template = @lookup_context.find("foo", %w(test), true) template = @lookup_context.find("foo", %w(test), true) @@ -221,12 +238,12 @@ class LookupContextWithFalseCaching < ActiveSupport::TestCase # Now we are going to change the template, but it won't change the returned template # since the timestamp is the same. - @resolver.hash["test/_foo.erb"][0] = "Bar" + @resolver.data["test/_foo.erb"][0] = "Bar" template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source # Now update the timestamp. - @resolver.hash["test/_foo.erb"][1] = Time.now.utc + @resolver.data["test/_foo.erb"][1] = Time.now.utc template = @lookup_context.find("foo", %w(test), true) assert_equal "Bar", template.source end @@ -237,7 +254,7 @@ class LookupContextWithFalseCaching < ActiveSupport::TestCase template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source - @resolver.hash.clear + @resolver.data.clear assert_raise ActionView::MissingTemplate do @lookup_context.find("foo", %w(test), true) end @@ -246,12 +263,12 @@ class LookupContextWithFalseCaching < ActiveSupport::TestCase test "if no template was cached in the first lookup, retrieval should work in the second call" do ActionView::Resolver.stub(:caching?, false) do - @resolver.hash.clear + @resolver.data.clear assert_raise ActionView::MissingTemplate do @lookup_context.find("foo", %w(test), true) end - @resolver.hash["test/_foo.erb"] = ["Foo", Time.utc(2000)] + @resolver.data["test/_foo.erb"] = ["Foo", Time.utc(2000)] template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source end diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index afe68b7ff0..ab67423ea1 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -15,7 +15,7 @@ module RenderTestCases def combined_fragment_cache_key(key) [ :views, key ] end - end.new(paths, @assigns) + end.with_view_paths(paths, @assigns) @controller_view = TestController.new.view_context diff --git a/actionview/test/template/streaming_render_test.rb b/actionview/test/template/streaming_render_test.rb index f196c42c4f..dda2095013 100644 --- a/actionview/test/template/streaming_render_test.rb +++ b/actionview/test/template/streaming_render_test.rb @@ -9,7 +9,7 @@ class SetupFiberedBase < ActiveSupport::TestCase def setup view_paths = ActionController::Base.view_paths @assigns = { secret: "in the sauce", name: nil } - @view = ActionView::Base.new(view_paths, @assigns) + @view = ActionView::Base.with_view_paths(view_paths, @assigns) @controller_view = TestController.new.view_context end diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index b348d1f17b..8257d97b7c 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -18,7 +18,7 @@ class TestERBTemplate < ActiveSupport::TestCase attr_accessor :formats end - class Context + class Context < ActionView::Base def initialize @output_buffer = "original" @virtual_path = nil diff --git a/actionview/test/template/test_case_test.rb b/actionview/test/template/test_case_test.rb index 976b6bc77e..ab3ababba4 100644 --- a/actionview/test/template/test_case_test.rb +++ b/actionview/test/template/test_case_test.rb @@ -52,7 +52,7 @@ module ActionView end test "retrieve non existing config values" do - assert_nil ActionView::Base.new.config.something_odd + assert_nil ActionView::Base.empty.config.something_odd end test "works without testing a helper module" do diff --git a/actionview/test/template/translation_helper_test.rb b/actionview/test/template/translation_helper_test.rb index e756348938..d04e68e182 100644 --- a/actionview/test/template/translation_helper_test.rb +++ b/actionview/test/template/translation_helper_test.rb @@ -36,7 +36,7 @@ class TranslationHelperTest < ActiveSupport::TestCase } } ) - @view = ::ActionView::Base.new(ActionController::Base.view_paths, {}) + @view = ::ActionView::Base.with_view_paths(ActionController::Base.view_paths, {}) end teardown do diff --git a/activejob/lib/active_job/enqueuing.rb b/activejob/lib/active_job/enqueuing.rb index ce118c1e8a..5609d13f5f 100644 --- a/activejob/lib/active_job/enqueuing.rb +++ b/activejob/lib/active_job/enqueuing.rb @@ -67,7 +67,7 @@ module ActiveJob false else ActiveSupport::Deprecation.warn( - "Rails 6.0 will return false when the enqueing is aborted. Make sure your code doesn't depend on it" \ + "Rails 6.0 will return false when the enqueuing is aborted. Make sure your code doesn't depend on it" \ " returning the instance of the job and set `config.active_job.return_false_on_aborted_enqueue = true`" \ " to remove the deprecations." ) diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index 046033921d..4d934df31b 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -1688,7 +1688,7 @@ class PerformedJobsTest < ActiveJob::TestCase end end - def test_assert_performed_with_without_bllock_with_global_id_args + def test_assert_performed_with_without_block_with_global_id_args ricardo = Person.new(9) HelloJob.perform_later(ricardo) perform_enqueued_jobs diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 571c050f2f..912b307683 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,31 @@ +* Fix year value when casting a multiparameter time hash + + When assigning a hash to a time attribute that's missing a year component + (e.g. a `time_select` with `:ignore_date` set to `true`) then the year + defaults to 1970 instead of the expected 2000. This results in the attribute + changing as a result of the save. + + Before: + ``` + event = Event.new(start_time: { 4 => 20, 5 => 30 }) + event.start_time # => 1970-01-01 20:30:00 UTC + event.save + event.reload + event.start_time # => 2000-01-01 20:30:00 UTC + ``` + + After: + ``` + event = Event.new(start_time: { 4 => 20, 5 => 30 }) + event.start_time # => 2000-01-01 20:30:00 UTC + event.save + event.reload + event.start_time # => 2000-01-01 20:30:00 UTC + ``` + + *Andrew White* + + ## Rails 6.0.0.beta1 (January 18, 2019) ## * Add `ActiveModel::Errors#of_kind?`. diff --git a/activemodel/lib/active_model/type/time.rb b/activemodel/lib/active_model/type/time.rb index b3056b1333..8fba01b86a 100644 --- a/activemodel/lib/active_model/type/time.rb +++ b/activemodel/lib/active_model/type/time.rb @@ -5,7 +5,7 @@ module ActiveModel class Time < Value # :nodoc: include Helpers::TimeValue include Helpers::AcceptsMultiparameterTime.new( - defaults: { 1 => 1970, 2 => 1, 3 => 1, 4 => 0, 5 => 0 } + defaults: { 1 => 2000, 2 => 1, 3 => 1, 4 => 0, 5 => 0 } ) def type diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index c5997283ea..51e224d5cd 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "bigdecimal/util" + module ActiveModel module Validations class NumericalityValidator < EachValidator # :nodoc: @@ -10,7 +12,6 @@ module ActiveModel RESERVED_OPTIONS = CHECKS.keys + [:only_integer] INTEGER_REGEX = /\A[+-]?\d+\z/ - DECIMAL_REGEX = /\A[+-]?\d+\.?\d*(e|e[+-])?\d+\z/ def check_validity! keys = CHECKS.keys - [:odd, :even] @@ -92,8 +93,8 @@ module ActiveModel raw_value elsif is_integer?(raw_value) raw_value.to_i - elsif is_decimal?(raw_value) && !is_hexadecimal_literal?(raw_value) - BigDecimal(raw_value) + elsif !is_hexadecimal_literal?(raw_value) + Kernel.Float(raw_value).to_d end end @@ -101,12 +102,8 @@ module ActiveModel INTEGER_REGEX.match?(raw_value.to_s) end - def is_decimal?(raw_value) - DECIMAL_REGEX.match?(raw_value.to_s) - end - def is_hexadecimal_literal?(raw_value) - /\A0[xX]/.match?(raw_value) + /\A0[xX]/.match?(raw_value.to_s) end def filtered_options(value) diff --git a/activemodel/test/cases/type/time_test.rb b/activemodel/test/cases/type/time_test.rb index 3fbae1a169..5c6271241d 100644 --- a/activemodel/test/cases/type/time_test.rb +++ b/activemodel/test/cases/type/time_test.rb @@ -16,6 +16,7 @@ module ActiveModel assert_equal ::Time.utc(2000, 1, 1, 16, 45, 54), type.cast("2015-06-13T19:45:54+03:00") assert_equal ::Time.utc(1999, 12, 31, 21, 7, 8), type.cast("06:07:08+09:00") + assert_equal ::Time.utc(2000, 1, 1, 16, 45, 54), type.cast(4 => 16, 5 => 45, 6 => 54) end def test_user_input_in_time_zone diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index eb4b02df93..fcdf123062 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -281,6 +281,19 @@ class NumericalityValidationTest < ActiveModel::TestCase assert_predicate topic, :invalid? end + def test_validates_numericalty_with_object_acting_as_numeric + klass = Class.new do + def to_f + 123.54 + end + end + + Topic.validates_numericality_of :price + topic = Topic.new(price: klass.new) + + assert_predicate topic, :valid? + end + def test_validates_numericality_with_invalid_args assert_raise(ArgumentError) { Topic.validates_numericality_of :approved, greater_than_or_equal_to: "foo" } assert_raise(ArgumentError) { Topic.validates_numericality_of :approved, less_than_or_equal_to: "foo" } diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 2a0cd81be5..654caafc92 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,45 @@ +* Allow applications to automatically switch connections. + + Adds a middleware and configuration options that can be used in your + application to automatically switch between the writing and reading + database connections. + + `GET` and `HEAD` requests will read from the replica unless there was + a write in the last 2 seconds, otherwise they will read from the primary. + Non-get requests will always write to the primary. The middleware accepts + an argument for a Resolver class and a Operations class where you are able + to change how the auto-switcher works to be most beneficial for your + application. + + To use the middleware in your application you can use the following + configuration options: + + ``` + config.active_record.database_selector = { delay: 2.seconds } + config.active_record.database_resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver + config.active_record.database_operations = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session + ``` + + To change the database selection strategy, pass a custom class to the + configuration options: + + ``` + config.active_record.database_selector = { delay: 10.seconds } + config.active_record.database_resolver = MyResolver + config.active_record.database_operations = MyResolver::MyCookies + ``` + + *Eileen M. Uchitelle* + +* MySQL: Support `:size` option to change text and blob size. + + *Ryuta Kamizono* + +* Make `t.timestamps` with precision by default. + + *Ryuta Kamizono* + + ## Rails 6.0.0.beta1 (January 18, 2019) ## * Remove deprecated `#set_state` from the transaction object. diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index de94f9693f..7d66158f47 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -74,6 +74,7 @@ module ActiveRecord autoload :Translation autoload :Validations autoload :SecureToken + autoload :DatabaseSelector, "active_record/middleware/database_selector" eager_autoload do autoload :ActiveRecordError, "active_record/errors" @@ -153,6 +154,12 @@ module ActiveRecord end end + module Middleware + extend ActiveSupport::Autoload + + autoload :DatabaseSelector, "active_record/middleware/database_selector" + end + module Tasks extend ActiveSupport::Autoload diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 99934a0e31..c8d5f679a8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -1006,7 +1006,16 @@ module ActiveRecord # for (not necessarily the current class). def retrieve_connection(spec_name) #:nodoc: pool = retrieve_connection_pool(spec_name) - raise ConnectionNotEstablished, "No connection pool with '#{spec_name}' found." unless pool + + unless pool + # multiple database application + if ActiveRecord::Base.connection_handler != ActiveRecord::Base.default_connection_handler + raise ConnectionNotEstablished, "No connection pool with '#{spec_name}' found for the '#{ActiveRecord::Base.current_role}' role." + else + raise ConnectionNotEstablished, "No connection pool with '#{spec_name}' found." + end + end + pool.connection end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index db489143af..28d37881b6 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -263,6 +263,7 @@ module ActiveRecord deprecate :indexes= def initialize( + conn, name, temporary: false, if_not_exists: false, @@ -271,6 +272,7 @@ module ActiveRecord comment: nil, ** ) + @conn = conn @columns_hash = {} @indexes = [] @foreign_keys = [] @@ -410,6 +412,10 @@ module ActiveRecord def timestamps(**options) options[:null] = false if options[:null].nil? + if !options.key?(:precision) && @conn.supports_datetime_with_precision? + options[:precision] = 6 + end + column(:created_at, :datetime, options) column(:updated_at, :datetime, options) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 208c8c9c64..d88e75d692 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -130,11 +130,11 @@ module ActiveRecord # column_exists?(:suppliers, :name, :string, null: false) # column_exists?(:suppliers, :tax, :decimal, precision: 8, scale: 2) # - def column_exists?(table_name, column_name, type = nil, options = {}) + def column_exists?(table_name, column_name, type = nil, **options) column_name = column_name.to_s checks = [] checks << lambda { |c| c.name == column_name } - checks << lambda { |c| c.type == type } if type + checks << lambda { |c| c.type == type.to_sym rescue nil } if type column_options_keys.each do |attr| checks << lambda { |c| c.send(attr) == options[attr] } if options.key?(attr) end @@ -1129,6 +1129,10 @@ module ActiveRecord def add_timestamps(table_name, options = {}) options[:null] = false if options[:null].nil? + if !options.key?(:precision) && supports_datetime_with_precision? + options[:precision] = 6 + end + add_column table_name, :created_at, :datetime, options add_column table_name, :updated_at, :datetime, options end @@ -1290,7 +1294,7 @@ module ActiveRecord end def create_table_definition(*args) - TableDefinition.new(*args) + TableDefinition.new(self, *args) end def create_alter_table(name) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 70d281b62b..7b69a63f6e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -29,7 +29,7 @@ module ActiveRecord NATIVE_DATABASE_TYPES = { primary_key: "bigint auto_increment PRIMARY KEY", string: { name: "varchar", limit: 255 }, - text: { name: "text", limit: 65535 }, + text: { name: "text" }, integer: { name: "int", limit: 4 }, float: { name: "float", limit: 24 }, decimal: { name: "decimal" }, @@ -37,7 +37,8 @@ module ActiveRecord timestamp: { name: "timestamp" }, time: { name: "time" }, date: { name: "date" }, - binary: { name: "blob", limit: 65535 }, + binary: { name: "blob" }, + blob: { name: "blob" }, boolean: { name: "tinyint", limit: 1 }, json: { name: "json" }, } @@ -627,6 +628,7 @@ module ActiveRecord ER_LOCK_WAIT_TIMEOUT = 1205 ER_QUERY_INTERRUPTED = 1317 ER_QUERY_TIMEOUT = 3024 + ER_FK_INCOMPATIBLE_COLUMNS = 3780 def translate_exception(exception, message:, sql:, binds:) case error_number(exception) @@ -634,7 +636,7 @@ module ActiveRecord RecordNotUnique.new(message, sql: sql, binds: binds) when ER_NO_REFERENCED_ROW, ER_ROW_IS_REFERENCED, ER_ROW_IS_REFERENCED_2, ER_NO_REFERENCED_ROW_2 InvalidForeignKey.new(message, sql: sql, binds: binds) - when ER_CANNOT_ADD_FOREIGN + when ER_CANNOT_ADD_FOREIGN, ER_FK_INCOMPATIBLE_COLUMNS mismatched_foreign_key(message, sql: sql, binds: binds) when ER_CANNOT_CREATE_TABLE if message.include?("errno: 150") @@ -708,6 +710,12 @@ module ActiveRecord end def add_timestamps_for_alter(table_name, options = {}) + options[:null] = false if options[:null].nil? + + if !options.key?(:precision) && supports_datetime_with_precision? + options[:precision] = 6 + end + [add_column_for_alter(table_name, :created_at, :datetime, options), add_column_for_alter(table_name, :updated_at, :datetime, options)] end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb index 2ed4ad16ae..90bcdf3297 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb @@ -56,6 +56,16 @@ module ActiveRecord case type when :virtual type = options[:type] + when :text, :blob, :binary + case (size = options[:size])&.to_s + when "tiny", "medium", "long" + sql_type = @conn.native_database_types[type][:name] + type = "#{size}#{sql_type}" + else + raise ArgumentError, <<~MSG unless size.nil? + #{size.inspect} is invalid :size value. Only :tiny, :medium, and :long are allowed. + MSG + end when :primary_key type = :integer options[:limit] ||= 8 diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb index d23178e43c..57518b02fa 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb @@ -10,6 +10,10 @@ module ActiveRecord spec[:unsigned] = "true" if column.unsigned? spec[:auto_increment] = "true" if column.auto_increment? + if /\A(?<size>tiny|medium|long)(?:text|blob)/ =~ column.sql_type + spec = { size: size.to_sym.inspect }.merge!(spec) + end + if @connection.supports_virtual_columns? && column.virtual? spec[:as] = extract_expression_for_virtual_column(column) spec[:stored] = "true" if /\b(?:STORED|PERSISTENT)\b/.match?(column.extra) @@ -37,13 +41,15 @@ module ActiveRecord case column.sql_type when /\Atimestamp\b/ :timestamp - when "tinyblob" - :blob else super end end + def schema_limit(column) + super unless /\A(?:tiny|medium|long)?(?:text|blob)/.match?(column.sql_type) + end + def schema_precision(column) super unless /\A(?:date)?time(?:stamp)?\b/.match?(column.sql_type) && column.precision == 0 end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index 47b5c4b9ec..e9484a08de 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -127,7 +127,7 @@ module ActiveRecord end def create_table_definition(*args) - MySQL::TableDefinition.new(*args) + MySQL::TableDefinition.new(self, *args) end def new_column_from_field(table_name, field) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb index ceb8b40bd9..84dd28907b 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb @@ -17,6 +17,42 @@ module ActiveRecord "VALIDATE CONSTRAINT #{quote_column_name(name)}" end + def visit_ChangeColumnDefinition(o) + column = o.column + column.sql_type = type_to_sql(column.type, column.options) + quoted_column_name = quote_column_name(o.name) + + change_column_sql = +"ALTER COLUMN #{quoted_column_name} TYPE #{column.sql_type}" + + options = column_options(column) + + if options[:collation] + change_column_sql << " COLLATE \"#{options[:collation]}\"" + end + + if options[:using] + change_column_sql << " USING #{options[:using]}" + elsif options[:cast_as] + cast_as_type = type_to_sql(options[:cast_as], options) + change_column_sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})" + end + + if options.key?(:default) + if options[:default].nil? + change_column_sql << ", ALTER COLUMN #{quoted_column_name} DROP DEFAULT" + else + quoted_default = quote_default_expression(options[:default], column) + change_column_sql << ", ALTER COLUMN #{quoted_column_name} SET DEFAULT #{quoted_default}" + end + end + + if options.key?(:null) + change_column_sql << ", ALTER COLUMN #{quoted_column_name} #{options[:null] ? 'DROP' : 'SET'} NOT NULL" + end + + change_column_sql + end + def add_column_options!(sql, options) if options[:collation] sql << " COLLATE \"#{options[:collation]}\"" diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 3516bef75a..946436f7f9 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -637,7 +637,7 @@ module ActiveRecord end def create_table_definition(*args) - PostgreSQL::TableDefinition.new(*args) + PostgreSQL::TableDefinition.new(self, *args) end def create_alter_table(name) @@ -683,38 +683,20 @@ module ActiveRecord end end - def change_column_sql(table_name, column_name, type, options = {}) - quoted_column_name = quote_column_name(column_name) - sql_type = type_to_sql(type, options) - sql = +"ALTER COLUMN #{quoted_column_name} TYPE #{sql_type}" - if options[:collation] - sql << " COLLATE \"#{options[:collation]}\"" - end - if options[:using] - sql << " USING #{options[:using]}" - elsif options[:cast_as] - cast_as_type = type_to_sql(options[:cast_as], options) - sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})" - end - - sql - end - def add_column_for_alter(table_name, column_name, type, options = {}) return super unless options.key?(:comment) [super, Proc.new { change_column_comment(table_name, column_name, options[:comment]) }] end def change_column_for_alter(table_name, column_name, type, options = {}) - sqls = [change_column_sql(table_name, column_name, type, options)] - sqls << change_column_default_for_alter(table_name, column_name, options[:default]) if options.key?(:default) - sqls << change_column_null_for_alter(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + td = create_table_definition(table_name) + cd = td.new_column_definition(column_name, type, options) + sqls = [schema_creation.accept(ChangeColumnDefinition.new(cd, column_name))] sqls << Proc.new { change_column_comment(table_name, column_name, options[:comment]) } if options.key?(:comment) sqls end - # Changes the default value of a table column. - def change_column_default_for_alter(table_name, column_name, default_or_changes) # :nodoc: + def change_column_default_for_alter(table_name, column_name, default_or_changes) column = column_for(table_name, column_name) return unless column @@ -729,11 +711,17 @@ module ActiveRecord end end - def change_column_null_for_alter(table_name, column_name, null, default = nil) #:nodoc: - "ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL" + def change_column_null_for_alter(table_name, column_name, null, default = nil) + "ALTER COLUMN #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL" end def add_timestamps_for_alter(table_name, options = {}) + options[:null] = false if options[:null].nil? + + if !options.key?(:precision) && supports_datetime_with_precision? + options[:precision] = 6 + end + [add_column_for_alter(table_name, :created_at, :datetime, options), add_column_for_alter(table_name, :updated_at, :datetime, options)] end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 381d5ab29b..95beeb4cae 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -236,6 +236,8 @@ module ActiveRecord # @local_tz is initialized as nil to avoid warnings when connect tries to use it @local_tz = nil + @default_timezone = nil + @timestamp_decoder = nil @max_identifier_length = nil configure_connection @@ -628,6 +630,10 @@ module ActiveRecord def exec_no_cache(sql, name, binds) materialize_transactions + # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been + # made since we established the connection + update_typemap_for_default_timezone + type_casted_binds = type_casted_binds(binds) log(sql, name, binds, type_casted_binds) do ActiveSupport::Dependencies.interlock.permit_concurrent_loads do @@ -638,6 +644,7 @@ module ActiveRecord def exec_cache(sql, name, binds) materialize_transactions + update_typemap_for_default_timezone stmt_key = prepare_statement(sql, binds) type_casted_binds = type_casted_binds(binds) @@ -826,6 +833,18 @@ module ActiveRecord @connection.type_map_for_queries = map end + def update_typemap_for_default_timezone + if @default_timezone != ActiveRecord::Base.default_timezone && @timestamp_decoder + decoder_class = ActiveRecord::Base.default_timezone == :utc ? + PG::TextDecoder::TimestampUtc : + PG::TextDecoder::TimestampWithoutTimeZone + + @timestamp_decoder = decoder_class.new(@timestamp_decoder.to_h) + @connection.type_map_for_results.add_coder(@timestamp_decoder) + @default_timezone = ActiveRecord::Base.default_timezone + end + end + def add_pg_decoders coders_by_name = { "int2" => PG::TextDecoder::Integer, @@ -836,6 +855,13 @@ module ActiveRecord "float8" => PG::TextDecoder::Float, "bool" => PG::TextDecoder::Boolean, } + + if defined?(PG::TextDecoder::TimestampUtc) + # Use native PG encoders available since pg-1.1 + coders_by_name["timestamp"] = PG::TextDecoder::TimestampUtc + coders_by_name["timestamptz"] = PG::TextDecoder::TimestampWithTimeZone + end + known_coder_types = coders_by_name.keys.map { |n| quote(n) } query = <<~SQL % known_coder_types.join(", ") SELECT t.oid, t.typname @@ -851,6 +877,10 @@ module ActiveRecord map = PG::TypeMapByOid.new coders.each { |coder| map.add_coder(coder) } @connection.type_map_for_results = map + + # extract timestamp decoder for use in update_typemap_for_default_timezone + @timestamp_decoder = coders.find { |coder| coder.name == "timestamp" } + update_typemap_for_default_timezone end def construct_coder(row, coder_class) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb index 8650c07bab..2394982a7d 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb @@ -62,7 +62,7 @@ module ActiveRecord end def create_table_definition(*args) - SQLite3::TableDefinition.new(*args) + SQLite3::TableDefinition.new(self, *args) end def new_column_from_field(table_name, field) diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index 4a941055d1..558cdeccf2 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -158,10 +158,6 @@ module ActiveRecord end def with_handler(handler_key, &blk) # :nodoc: - unless ActiveRecord::Base.connection_handlers.keys.include?(handler_key) - raise ArgumentError, "The #{handler_key} role does not exist. Add it by establishing a connection with `connects_to` or use an existing role (#{ActiveRecord::Base.connection_handlers.keys.join(", ")})." - end - handler = lookup_connection_handler(handler_key) swap_connection_handler(handler, &blk) end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 369d63e40a..519acd7605 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -101,6 +101,7 @@ module ActiveRecord # environment where dumping schema is rarely needed. mattr_accessor :dump_schema_after_migration, instance_writer: false, default: true + mattr_accessor :database_selector, instance_writer: false ## # :singleton-method: # Specifies which database schemas to dump when calling db:structure:dump. diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index 11aed6c002..73adf66684 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -134,9 +134,11 @@ module ActiveRecord end def build_db_config_from_hash(env_name, spec_name, config) - if url = config["url"] + if config.has_key?("url") + url = config["url"] config_without_url = config.dup config_without_url.delete "url" + ActiveRecord::DatabaseConfigurations::UrlConfig.new(env_name, spec_name, url, config_without_url) elsif config["database"] || (config.size == 1 && config.values.all? { |v| v.is_a? String }) ActiveRecord::DatabaseConfigurations::HashConfig.new(env_name, spec_name, config) diff --git a/activerecord/lib/active_record/database_configurations/url_config.rb b/activerecord/lib/active_record/database_configurations/url_config.rb index 81917fc4c1..8e8aa69478 100644 --- a/activerecord/lib/active_record/database_configurations/url_config.rb +++ b/activerecord/lib/active_record/database_configurations/url_config.rb @@ -56,12 +56,17 @@ module ActiveRecord end private - def build_config(original_config, url) - if /^jdbc:/.match?(url) - hash = { "url" => url } + + def build_url_hash(url) + if url.nil? || /^jdbc:/.match?(url) + { "url" => url } else - hash = ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(url).to_hash + ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(url).to_hash end + end + + def build_config(original_config, url) + hash = build_url_hash(url) if original_config[env_name] original_config[env_name].merge(hash) diff --git a/activerecord/lib/active_record/middleware/database_selector.rb b/activerecord/lib/active_record/middleware/database_selector.rb new file mode 100644 index 0000000000..06529bc1c9 --- /dev/null +++ b/activerecord/lib/active_record/middleware/database_selector.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require "active_record/middleware/database_selector/resolver" + +module ActiveRecord + module Middleware + # The DatabaseSelector Middleware provides a framework for automatically + # swapping from the primary to the replica database connection. Rails + # provides a basic framework to determine when to swap and allows for + # applications to write custom strategy classes to override the default + # behavior. + # + # The resolver class defines when the application should switch (i.e. read + # from the primary if a write occurred less than 2 seconds ago) and an + # operations class that sets a value that helps the resolver class decide + # when to switch. + # + # Rails default middleware uses the request's session to set a timestamp + # that informs the application when to read from a primary or read from a + # replica. + # + # To use the DatabaseSelector in your application with default settings add + # the following options to your environment config: + # + # config.active_record.database_selector = { delay: 2.seconds } + # config.active_record.database_resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver + # config.active_record.database_operations = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session + # + # New applications will include these lines commented out in the production.rb. + # + # The default behavior can be changed by setting the config options to a + # custom class: + # + # config.active_record.database_selector = { delay: 2.seconds } + # config.active_record.database_resolver = MyResolver + # config.active_record.database_operations = MyResolver::MySession + class DatabaseSelector + def initialize(app, resolver_klass = Resolver, operations_klass = Resolver::Session) + @app = app + @resolver_klass = resolver_klass + @operations_klass = operations_klass + end + + attr_reader :resolver_klass, :operations_klass + + # Middleware that determines which database connection to use in a mutliple + # database application. + def call(env) + request = ActionDispatch::Request.new(env) + + select_database(request) do + @app.call(env) + end + end + + private + + def select_database(request, &blk) + operations = operations_klass.build(request) + database_resolver = resolver_klass.call(operations) + + if reading_request?(request) + database_resolver.read(&blk) + else + database_resolver.write(&blk) + end + end + + def reading_request?(request) + request.get? || request.head? + end + end + end +end diff --git a/activerecord/lib/active_record/middleware/database_selector/resolver.rb b/activerecord/lib/active_record/middleware/database_selector/resolver.rb new file mode 100644 index 0000000000..1c056e2156 --- /dev/null +++ b/activerecord/lib/active_record/middleware/database_selector/resolver.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +require "active_record/middleware/database_selector/resolver/session" + +module ActiveRecord + module Middleware + class DatabaseSelector + # The Resolver class is used by the DatabaseSelector middleware to + # determine which database the request should use. + # + # To change the behavior of the Resolver class in your application, + # create a custom resolver class that inherts from + # DatabaseSelector::Resolver and implements the methods that need to + # be changed. + # + # By default the Resolver class will send read traffic to the replica + # if it's been 2 seconds since the last write. + class Resolver # :nodoc: + SEND_TO_REPLICA_DELAY = 2.seconds + + def self.call(resolver) + new(resolver) + end + + def initialize(resolver) + @resolver = resolver + @instrumenter = ActiveSupport::Notifications.instrumenter + end + + attr_reader :resolver, :instrumenter + + def read(&blk) + if read_from_primary? + read_from_primary(&blk) + else + read_from_replica(&blk) + end + end + + def write(&blk) + write_to_primary(&blk) + end + + private + + def read_from_primary(&blk) + ActiveRecord::Base.connection.while_preventing_writes do + ActiveRecord::Base.connected_to(role: :writing) do + instrumenter.instrument("database_selector.active_record.read_from_primary") do + yield + end + end + end + end + + def read_from_replica(&blk) + ActiveRecord::Base.connected_to(role: :reading) do + instrumenter.instrument("database_selector.active_record.read_from_replica") do + yield + end + end + end + + def write_to_primary(&blk) + ActiveRecord::Base.connected_to(role: :writing) do + instrumenter.instrument("database_selector.active_record.wrote_to_primary") do + resolver.update_last_write_timestamp + yield + end + end + end + + def read_from_primary? + !time_since_last_write_ok? + end + + def send_to_replica_delay + (ActiveRecord::Base.database_selector && ActiveRecord::Base.database_selector[:delay]) || + SEND_TO_REPLICA_DELAY + end + + def time_since_last_write_ok? + Time.now - resolver.last_write_timestamp >= send_to_replica_delay + end + end + end + end +end diff --git a/activerecord/lib/active_record/middleware/database_selector/resolver/session.rb b/activerecord/lib/active_record/middleware/database_selector/resolver/session.rb new file mode 100644 index 0000000000..33e0af5ee4 --- /dev/null +++ b/activerecord/lib/active_record/middleware/database_selector/resolver/session.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module ActiveRecord + module Middleware + class DatabaseSelector + class Resolver + # The session class is used by the DatabaseSelector::Resolver to save + # timestamps of the last write in the session. + # + # The last_write is used to determine whether it's safe to read + # from the replica or the request needs to be sent to the primary. + class Session # :nodoc: + def self.build(request) + new(request.session) + end + + # Converts time to a timestamp that represents milliseconds since + # epoch. + def self.convert_time_to_timestamp(time) + time.to_i * 1000 + time.usec / 1000 + end + + # Converts milliseconds since epoch timestamp into a time object. + def self.convert_timestamp_to_time(timestamp) + timestamp ? Time.at(timestamp / 1000, (timestamp % 1000) * 1000) : Time.at(0) + end + + def initialize(session) + @session = session + end + + attr_reader :session + + def last_write_timestamp + self.class.convert_timestamp_to_time(session[:last_write]) + end + + def update_last_write_timestamp + session[:last_write] = self.class.convert_time_to_timestamp(Time.now) + end + end + end + end + end +end diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 8f6fcfcaea..94906a2943 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -16,13 +16,55 @@ module ActiveRecord V6_0 = Current class V5_2 < V6_0 + module TableDefinition + def timestamps(**options) + options[:precision] ||= nil + super + end + end + module CommandRecorder def invert_transaction(args, &block) [:transaction, args, block] end end + def create_table(table_name, **options) + if block_given? + super { |t| yield compatible_table_definition(t) } + else + super + end + end + + def change_table(table_name, **options) + if block_given? + super { |t| yield compatible_table_definition(t) } + else + super + end + end + + def create_join_table(table_1, table_2, **options) + if block_given? + super { |t| yield compatible_table_definition(t) } + else + super + end + end + + def add_timestamps(table_name, **options) + options[:precision] ||= nil + super + end + private + def compatible_table_definition(t) + class << t + prepend TableDefinition + end + t + end def command_recorder recorder = super @@ -36,9 +78,7 @@ module ActiveRecord class V5_1 < V5_2 def change_column(table_name, column_name, type, options = {}) if adapter_name == "PostgreSQL" - clear_cache! - sql = connection.send(:change_column_sql, table_name, column_name, type, options) - execute "ALTER TABLE #{quote_table_name(table_name)} #{sql}" + super(table_name, column_name, type, options.except(:default, :null, :comment)) change_column_default(table_name, column_name, options[:default]) if options.key?(:default) change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) @@ -89,35 +129,12 @@ module ActiveRecord options[:id] = :integer end - if block_given? - super do |t| - yield compatible_table_definition(t) - end - else - super - end - end - - def change_table(table_name, options = {}) - if block_given? - super do |t| - yield compatible_table_definition(t) - end - else - super - end + super end def create_join_table(table_1, table_2, column_options: {}, **options) column_options.reverse_merge!(type: :integer) - - if block_given? - super do |t| - yield compatible_table_definition(t) - end - else - super - end + super end def add_column(table_name, column_name, type, options = {}) @@ -138,7 +155,7 @@ module ActiveRecord class << t prepend TableDefinition end - t + super end end @@ -156,33 +173,13 @@ module ActiveRecord end end - def create_table(table_name, options = {}) - if block_given? - super do |t| - yield compatible_table_definition(t) - end - else - super - end - end - - def change_table(table_name, options = {}) - if block_given? - super do |t| - yield compatible_table_definition(t) - end - else - super - end - end - - def add_reference(*, **options) + def add_reference(table_name, ref_name, **options) options[:index] ||= false super end alias :add_belongs_to :add_reference - def add_timestamps(_, **options) + def add_timestamps(table_name, **options) options[:null] = true if options[:null].nil? super end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index c2b60610ce..2213fbefb4 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -436,7 +436,7 @@ module ActiveRecord end alias update_attributes update - deprecate :update_attributes + deprecate update_attributes: "please, use update instead" # Updates its receiver just like #update but calls #save! instead # of +save+, so an exception is raised if the record is invalid and saving will fail. @@ -450,7 +450,7 @@ module ActiveRecord end alias update_attributes! update! - deprecate :update_attributes! + deprecate update_attributes!: "please, use update! instead" # Equivalent to <code>update_columns(name => value)</code>. def update_column(name, value) diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 6346a95d57..a981fa97d9 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -88,6 +88,14 @@ module ActiveRecord end end + initializer "active_record.database_selector" do + if config.active_record.database_selector + resolver = config.active_record.delete(:database_resolver) + operations = config.active_record.delete(:database_operations) + config.app_middleware.use ActiveRecord::Middleware::DatabaseSelector, resolver, operations + end + end + initializer "Check for cache versioning support" do config.after_initialize do |app| ActiveSupport.on_load(:active_record) do diff --git a/activerecord/lib/active_record/railties/collection_cache_association_loading.rb b/activerecord/lib/active_record/railties/collection_cache_association_loading.rb index b5129e4239..dfaac4eefb 100644 --- a/activerecord/lib/active_record/railties/collection_cache_association_loading.rb +++ b/activerecord/lib/active_record/railties/collection_cache_association_loading.rb @@ -20,12 +20,12 @@ module ActiveRecord end end - def collection_without_template + def collection_without_template(*) @relation.preload_associations(@collection) if @relation super end - def collection_with_template + def collection_with_template(*) @relation.preload_associations(@collection) if @relation super end diff --git a/activerecord/lib/active_record/relation/query_attribute.rb b/activerecord/lib/active_record/relation/query_attribute.rb index 5e0b4ac160..1dd6462d8d 100644 --- a/activerecord/lib/active_record/relation/query_attribute.rb +++ b/activerecord/lib/active_record/relation/query_attribute.rb @@ -32,7 +32,7 @@ module ActiveRecord if defined?(@_unboundable) @_unboundable else - value_for_database + value_for_database unless value_before_type_cast.is_a?(StatementCache::Substitute) @_unboundable = nil end rescue ::RangeError diff --git a/activerecord/lib/arel/nodes/and.rb b/activerecord/lib/arel/nodes/and.rb index c530a77bfb..bf516db35f 100644 --- a/activerecord/lib/arel/nodes/and.rb +++ b/activerecord/lib/arel/nodes/and.rb @@ -2,7 +2,7 @@ module Arel # :nodoc: all module Nodes - class And < Arel::Nodes::Node + class And < Arel::Nodes::NodeExpression attr_reader :children def initialize(children) diff --git a/activerecord/lib/arel/nodes/case.rb b/activerecord/lib/arel/nodes/case.rb index 654a54825e..1c4b727bf6 100644 --- a/activerecord/lib/arel/nodes/case.rb +++ b/activerecord/lib/arel/nodes/case.rb @@ -2,7 +2,7 @@ module Arel # :nodoc: all module Nodes - class Case < Arel::Nodes::Node + class Case < Arel::Nodes::NodeExpression attr_accessor :case, :conditions, :default def initialize(expression = nil, default = nil) diff --git a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb index 2d71ee2f15..88c2ac5d0a 100644 --- a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb +++ b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb @@ -139,8 +139,8 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase with_real_execute do ActiveRecord::Base.connection.create_table :delete_me ActiveRecord::Base.connection.add_timestamps :delete_me, null: true - assert column_present?("delete_me", "updated_at", "datetime") - assert column_present?("delete_me", "created_at", "datetime") + assert column_exists?("delete_me", "updated_at", "datetime") + assert column_exists?("delete_me", "created_at", "datetime") ensure ActiveRecord::Base.connection.drop_table :delete_me rescue nil end @@ -152,8 +152,8 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase t.timestamps null: true end ActiveRecord::Base.connection.remove_timestamps :delete_me, null: true - assert_not column_present?("delete_me", "updated_at", "datetime") - assert_not column_present?("delete_me", "created_at", "datetime") + assert_not column_exists?("delete_me", "updated_at", "datetime") + assert_not column_exists?("delete_me", "created_at", "datetime") ensure ActiveRecord::Base.connection.drop_table :delete_me rescue nil end @@ -194,9 +194,4 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase def method_missing(method_symbol, *arguments) ActiveRecord::Base.connection.send(method_symbol, *arguments) end - - def column_present?(table_name, column_name, type) - results = ActiveRecord::Base.connection.select_all("SHOW FIELDS FROM #{table_name} LIKE '#{column_name}'") - results.first && results.first["Type"] == type - end end diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index b12055a40a..2e7a4b498f 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -17,7 +17,7 @@ class PostgresqlArrayTest < ActiveRecord::PostgreSQLTestCase enable_extension!("hstore", @connection) @connection.transaction do - @connection.create_table("pg_arrays") do |t| + @connection.create_table "pg_arrays", force: true do |t| t.string "tags", array: true, limit: 255 t.integer "ratings", array: true t.datetime :datetimes, array: true @@ -112,6 +112,18 @@ class PostgresqlArrayTest < ActiveRecord::PostgreSQLTestCase assert_predicate column, :array? end + def test_change_column_from_non_array_to_array + @connection.add_column :pg_arrays, :snippets, :string + @connection.change_column :pg_arrays, :snippets, :text, array: true, default: [], using: "string_to_array(\"snippets\", ',')" + + PgArray.reset_column_information + column = PgArray.columns_hash["snippets"] + + assert_equal :text, column.type + assert_equal [], PgArray.column_defaults["snippets"] + assert_predicate column, :array? + end + def test_change_column_cant_make_non_array_column_to_array @connection.add_column :pg_arrays, :a_string, :string assert_raises ActiveRecord::StatementInvalid do diff --git a/activerecord/test/cases/ar_schema_test.rb b/activerecord/test/cases/ar_schema_test.rb index f05dcac7dd..9d88b14dab 100644 --- a/activerecord/test/cases/ar_schema_test.rb +++ b/activerecord/test/cases/ar_schema_test.rb @@ -116,8 +116,8 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase end end - assert_not @connection.columns(:has_timestamps).find { |c| c.name == "created_at" }.null - assert_not @connection.columns(:has_timestamps).find { |c| c.name == "updated_at" }.null + assert @connection.column_exists?(:has_timestamps, :created_at, null: false) + assert @connection.column_exists?(:has_timestamps, :updated_at, null: false) end def test_timestamps_without_null_set_null_to_false_on_change_table @@ -129,8 +129,23 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase end end - assert_not @connection.columns(:has_timestamps).find { |c| c.name == "created_at" }.null - assert_not @connection.columns(:has_timestamps).find { |c| c.name == "updated_at" }.null + assert @connection.column_exists?(:has_timestamps, :created_at, null: false) + assert @connection.column_exists?(:has_timestamps, :updated_at, null: false) + end + + if ActiveRecord::Base.connection.supports_bulk_alter? + def test_timestamps_without_null_set_null_to_false_on_change_table_with_bulk + ActiveRecord::Schema.define do + create_table :has_timestamps + + change_table :has_timestamps, bulk: true do |t| + t.timestamps default: Time.now + end + end + + assert @connection.column_exists?(:has_timestamps, :created_at, null: false) + assert @connection.column_exists?(:has_timestamps, :updated_at, null: false) + end end def test_timestamps_without_null_set_null_to_false_on_add_timestamps @@ -139,7 +154,58 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase add_timestamps :has_timestamps, default: Time.now end - assert_not @connection.columns(:has_timestamps).find { |c| c.name == "created_at" }.null - assert_not @connection.columns(:has_timestamps).find { |c| c.name == "updated_at" }.null + assert @connection.column_exists?(:has_timestamps, :created_at, null: false) + assert @connection.column_exists?(:has_timestamps, :updated_at, null: false) + end + + if subsecond_precision_supported? + def test_timestamps_sets_presicion_on_create_table + ActiveRecord::Schema.define do + create_table :has_timestamps do |t| + t.timestamps + end + end + + assert @connection.column_exists?(:has_timestamps, :created_at, precision: 6, null: false) + assert @connection.column_exists?(:has_timestamps, :updated_at, precision: 6, null: false) + end + + def test_timestamps_sets_presicion_on_change_table + ActiveRecord::Schema.define do + create_table :has_timestamps + + change_table :has_timestamps do |t| + t.timestamps default: Time.now + end + end + + assert @connection.column_exists?(:has_timestamps, :created_at, precision: 6, null: false) + assert @connection.column_exists?(:has_timestamps, :updated_at, precision: 6, null: false) + end + + if ActiveRecord::Base.connection.supports_bulk_alter? + def test_timestamps_sets_presicion_on_change_table_with_bulk + ActiveRecord::Schema.define do + create_table :has_timestamps + + change_table :has_timestamps, bulk: true do |t| + t.timestamps default: Time.now + end + end + + assert @connection.column_exists?(:has_timestamps, :created_at, precision: 6, null: false) + assert @connection.column_exists?(:has_timestamps, :updated_at, precision: 6, null: false) + end + end + + def test_timestamps_sets_presicion_on_add_timestamps + ActiveRecord::Schema.define do + create_table :has_timestamps + add_timestamps :has_timestamps, default: Time.now + end + + assert @connection.column_exists?(:has_timestamps, :created_at, precision: 6, null: false) + assert @connection.column_exists?(:has_timestamps, :updated_at, precision: 6, null: false) + end end end diff --git a/activerecord/test/cases/arel/nodes/and_test.rb b/activerecord/test/cases/arel/nodes/and_test.rb index eff54abd91..d123ca9fd0 100644 --- a/activerecord/test/cases/arel/nodes/and_test.rb +++ b/activerecord/test/cases/arel/nodes/and_test.rb @@ -16,6 +16,15 @@ module Arel assert_equal 2, array.uniq.size end end + + describe "functions as node expression" do + it "allows aliasing" do + aliased = And.new(["foo", "bar"]).as("baz") + + assert_kind_of As, aliased + assert_kind_of SqlLiteral, aliased.right + end + end end end end diff --git a/activerecord/test/cases/arel/nodes/case_test.rb b/activerecord/test/cases/arel/nodes/case_test.rb index 89861488df..946c2b0453 100644 --- a/activerecord/test/cases/arel/nodes/case_test.rb +++ b/activerecord/test/cases/arel/nodes/case_test.rb @@ -80,6 +80,16 @@ module Arel assert_equal 2, array.uniq.size end end + + describe "#as" do + it "allows aliasing" do + node = Case.new "foo" + as = node.as("bar") + + assert_equal node, as.left + assert_kind_of Arel::Nodes::SqlLiteral, as.right + end + end end end end diff --git a/activerecord/test/cases/cache_key_test.rb b/activerecord/test/cases/cache_key_test.rb index 3a06b1c795..c27eb8a65d 100644 --- a/activerecord/test/cases/cache_key_test.rb +++ b/activerecord/test/cases/cache_key_test.rb @@ -51,7 +51,7 @@ module ActiveRecord end test "cache_version is the same when it comes from the DB or from the user" do - skip("Mysql2 does not return a string value for updated_at") if current_adapter?(:Mysql2Adapter) + skip("Mysql2 and PostgreSQL don't return a string value for updated_at") if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) record = CacheMeWithVersion.create record_from_db = CacheMeWithVersion.find(record.id) @@ -63,7 +63,7 @@ module ActiveRecord end test "cache_version does not truncate zeros when timestamp ends in zeros" do - skip("Mysql2 does not return a string value for updated_at") if current_adapter?(:Mysql2Adapter) + skip("Mysql2 and PostgreSQL don't return a string value for updated_at") if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) travel_to Time.now.beginning_of_day do record = CacheMeWithVersion.create @@ -84,7 +84,7 @@ module ActiveRecord end test "cache_version does NOT call updated_at when value is from the database" do - skip("Mysql2 does not return a string value for updated_at") if current_adapter?(:Mysql2Adapter) + skip("Mysql2 and PostgreSQL don't return a string value for updated_at") if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) record = CacheMeWithVersion.create record_from_db = CacheMeWithVersion.find(record.id) diff --git a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb index 865aacc1b5..8988755d24 100644 --- a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb @@ -336,13 +336,13 @@ module ActiveRecord end def test_calling_connected_to_on_a_non_existent_handler_raises - error = assert_raises ArgumentError do + error = assert_raises ActiveRecord::ConnectionNotEstablished do ActiveRecord::Base.connected_to(role: :reading) do - yield + Person.first end end - assert_equal "The reading role does not exist. Add it by establishing a connection with `connects_to` or use an existing role (writing).", error.message + assert_equal "No connection pool with 'primary' found for the 'reading' role.", error.message end end end diff --git a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb index 06c1c51724..225cccc62c 100644 --- a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb +++ b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb @@ -46,6 +46,14 @@ module ActiveRecord assert_equal expected, actual end + def test_resolver_with_nil_database_url_and_current_env + ENV["RAILS_ENV"] = "foo" + config = { "foo" => { "adapter" => "postgres", "url" => ENV["DATABASE_URL"] } } + actual = resolve_spec(:foo, config) + expected = { "adapter" => "postgres", "url" => nil, "name" => "foo" } + assert_equal expected, actual + end + def test_resolver_with_database_uri_and_current_env_symbol_key_and_rack_env ENV["DATABASE_URL"] = "postgres://localhost/foo" ENV["RACK_ENV"] = "foo" diff --git a/activerecord/test/cases/database_selector_test.rb b/activerecord/test/cases/database_selector_test.rb new file mode 100644 index 0000000000..f961fa754b --- /dev/null +++ b/activerecord/test/cases/database_selector_test.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/person" +require "action_dispatch" + +module ActiveRecord + class DatabaseSelectorTest < ActiveRecord::TestCase + setup do + @session_store = {} + @session = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session.new(@session_store) + end + + def test_empty_session + assert_equal Time.at(0), @session.last_write_timestamp + end + + def test_writing_the_session_timestamps + assert @session.update_last_write_timestamp + + session2 = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session.new(@session_store) + assert_equal @session.last_write_timestamp, session2.last_write_timestamp + end + + def test_writing_session_time_changes + assert @session.update_last_write_timestamp + + before = @session.last_write_timestamp + sleep(0.1) + + assert @session.update_last_write_timestamp + assert_not_equal before, @session.last_write_timestamp + end + + def test_read_from_replicas + @session_store[:last_write] = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session.convert_time_to_timestamp(Time.now - 5.seconds) + + resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver.new(@session) + + called = false + resolver.read do + called = true + assert ActiveRecord::Base.connected_to?(role: :reading) + end + assert called + end + + def test_read_from_primary + @session_store[:last_write] = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session.convert_time_to_timestamp(Time.now) + + resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver.new(@session) + + called = false + resolver.read do + called = true + assert ActiveRecord::Base.connected_to?(role: :writing) + end + assert called + end + + def test_the_middleware_chooses_writing_role_with_POST_request + middleware = ActiveRecord::Middleware::DatabaseSelector.new(lambda { |env| + assert ActiveRecord::Base.connected_to?(role: :writing) + [200, {}, ["body"]] + }) + assert_equal [200, {}, ["body"]], middleware.call("REQUEST_METHOD" => "POST") + end + + def test_the_middleware_chooses_reading_role_with_GET_request + middleware = ActiveRecord::Middleware::DatabaseSelector.new(lambda { |env| + assert ActiveRecord::Base.connected_to?(role: :reading) + [200, {}, ["body"]] + }) + assert_equal [200, {}, ["body"]], middleware.call("REQUEST_METHOD" => "GET") + end + end +end diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 017ee7951e..5753bd7117 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -86,8 +86,8 @@ module ActiveRecord ActiveRecord::Migrator.new(:up, [migration]).migrate - assert connection.columns(:more_testings).find { |c| c.name == "created_at" }.null - assert connection.columns(:more_testings).find { |c| c.name == "updated_at" }.null + assert connection.column_exists?(:more_testings, :created_at, null: true) + assert connection.column_exists?(:more_testings, :updated_at, null: true) ensure connection.drop_table :more_testings rescue nil end @@ -103,8 +103,25 @@ module ActiveRecord ActiveRecord::Migrator.new(:up, [migration]).migrate - assert connection.columns(:testings).find { |c| c.name == "created_at" }.null - assert connection.columns(:testings).find { |c| c.name == "updated_at" }.null + assert connection.column_exists?(:testings, :created_at, null: true) + assert connection.column_exists?(:testings, :updated_at, null: true) + end + + if ActiveRecord::Base.connection.supports_bulk_alter? + def test_timestamps_have_null_constraints_if_not_present_in_migration_of_change_table_with_bulk + migration = Class.new(ActiveRecord::Migration[4.2]) { + def migrate(x) + change_table :testings, bulk: true do |t| + t.timestamps + end + end + }.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + + assert connection.column_exists?(:testings, :created_at, null: true) + assert connection.column_exists?(:testings, :updated_at, null: true) + end end def test_timestamps_have_null_constraints_if_not_present_in_migration_for_adding_timestamps_to_existing_table @@ -116,8 +133,70 @@ module ActiveRecord ActiveRecord::Migrator.new(:up, [migration]).migrate - assert connection.columns(:testings).find { |c| c.name == "created_at" }.null - assert connection.columns(:testings).find { |c| c.name == "updated_at" }.null + assert connection.column_exists?(:testings, :created_at, null: true) + assert connection.column_exists?(:testings, :updated_at, null: true) + end + + def test_timestamps_doesnt_set_precision_on_create_table + migration = Class.new(ActiveRecord::Migration[5.2]) { + def migrate(x) + create_table :more_testings do |t| + t.timestamps + end + end + }.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + + assert connection.column_exists?(:more_testings, :created_at, null: false, **precision_implicit_default) + assert connection.column_exists?(:more_testings, :updated_at, null: false, **precision_implicit_default) + ensure + connection.drop_table :more_testings rescue nil + end + + def test_timestamps_doesnt_set_precision_on_change_table + migration = Class.new(ActiveRecord::Migration[5.2]) { + def migrate(x) + change_table :testings do |t| + t.timestamps default: Time.now + end + end + }.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + + assert connection.column_exists?(:testings, :created_at, null: false, **precision_implicit_default) + assert connection.column_exists?(:testings, :updated_at, null: false, **precision_implicit_default) + end + + if ActiveRecord::Base.connection.supports_bulk_alter? + def test_timestamps_doesnt_set_precision_on_change_table_with_bulk + migration = Class.new(ActiveRecord::Migration[5.2]) { + def migrate(x) + change_table :testings, bulk: true do |t| + t.timestamps + end + end + }.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + + assert connection.column_exists?(:testings, :created_at, null: false, **precision_implicit_default) + assert connection.column_exists?(:testings, :updated_at, null: false, **precision_implicit_default) + end + end + + def test_timestamps_doesnt_set_precision_on_add_timestamps + migration = Class.new(ActiveRecord::Migration[5.2]) { + def migrate(x) + add_timestamps :testings, default: Time.now + end + }.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + + assert connection.column_exists?(:testings, :created_at, null: false, **precision_implicit_default) + assert connection.column_exists?(:testings, :updated_at, null: false, **precision_implicit_default) end def test_legacy_migrations_raises_exception_when_inherited @@ -159,6 +238,15 @@ module ActiveRecord ActiveRecord::Base.clear_cache! end end + + private + def precision_implicit_default + if current_adapter?(:Mysql2Adapter) + { presicion: 0 } + else + { presicion: nil } + end + end end end end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 46e2ff79d9..02031e51ef 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -626,6 +626,18 @@ class MigrationTest < ActiveRecord::TestCase ensure Person.connection.drop_table :test_text_limits, if_exists: true end + + def test_invalid_text_size_should_raise + e = assert_raise(ArgumentError) do + Person.connection.create_table :test_text_sizes, force: true do |t| + t.text :bigtext, size: 0xfffffffff + end + end + + assert_match(/#{0xfffffffff} is invalid :size value\. Only :tiny, :medium, and :long are allowed\./, e.message) + ensure + Person.connection.drop_table :test_text_sizes, if_exists: true + end end if ActiveRecord::Base.connection.supports_advisory_locks? diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index dda3efa47c..49e9be9565 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -245,25 +245,31 @@ class SchemaDumperTest < ActiveRecord::TestCase if current_adapter?(:Mysql2Adapter) def test_schema_dump_includes_length_for_mysql_binary_fields - output = standard_dump + output = dump_table_schema "binary_fields" assert_match %r{t\.binary\s+"var_binary",\s+limit: 255$}, output assert_match %r{t\.binary\s+"var_binary_large",\s+limit: 4095$}, output end def test_schema_dump_includes_length_for_mysql_blob_and_text_fields - output = standard_dump - assert_match %r{t\.blob\s+"tiny_blob",\s+limit: 255$}, output + output = dump_table_schema "binary_fields" + assert_match %r{t\.binary\s+"tiny_blob",\s+size: :tiny$}, output assert_match %r{t\.binary\s+"normal_blob"$}, output - assert_match %r{t\.binary\s+"medium_blob",\s+limit: 16777215$}, output - assert_match %r{t\.binary\s+"long_blob",\s+limit: 4294967295$}, output - assert_match %r{t\.text\s+"tiny_text",\s+limit: 255$}, output + assert_match %r{t\.binary\s+"medium_blob",\s+size: :medium$}, output + assert_match %r{t\.binary\s+"long_blob",\s+size: :long$}, output + assert_match %r{t\.text\s+"tiny_text",\s+size: :tiny$}, output assert_match %r{t\.text\s+"normal_text"$}, output - assert_match %r{t\.text\s+"medium_text",\s+limit: 16777215$}, output - assert_match %r{t\.text\s+"long_text",\s+limit: 4294967295$}, output + assert_match %r{t\.text\s+"medium_text",\s+size: :medium$}, output + assert_match %r{t\.text\s+"long_text",\s+size: :long$}, output + assert_match %r{t\.binary\s+"tiny_blob_2",\s+size: :tiny$}, output + assert_match %r{t\.binary\s+"medium_blob_2",\s+size: :medium$}, output + assert_match %r{t\.binary\s+"long_blob_2",\s+size: :long$}, output + assert_match %r{t\.text\s+"tiny_text_2",\s+size: :tiny$}, output + assert_match %r{t\.text\s+"medium_text_2",\s+size: :medium$}, output + assert_match %r{t\.text\s+"long_text_2",\s+size: :long$}, output end def test_schema_does_not_include_limit_for_emulated_mysql_boolean_fields - output = standard_dump + output = dump_table_schema "booleans" assert_no_match %r{t\.boolean\s+"has_fun",.+limit: 1}, output end diff --git a/activerecord/test/cases/statement_cache_test.rb b/activerecord/test/cases/statement_cache_test.rb index e3c12f68fd..6a6d73dc38 100644 --- a/activerecord/test/cases/statement_cache_test.rb +++ b/activerecord/test/cases/statement_cache_test.rb @@ -4,6 +4,7 @@ require "cases/helper" require "models/book" require "models/liquid" require "models/molecule" +require "models/numeric_data" require "models/electron" module ActiveRecord @@ -74,6 +75,11 @@ module ActiveRecord assert_equal "salty", liquids[0].name end + def test_statement_cache_with_strictly_cast_attribute + row = NumericData.create(temperature: 1.5) + assert_equal row, NumericData.find_by(temperature: 1.5) + end + def test_statement_cache_values_differ cache = ActiveRecord::StatementCache.create(Book.connection) do |params| Book.where(name: "my book") diff --git a/activerecord/test/cases/type/time_test.rb b/activerecord/test/cases/type/time_test.rb new file mode 100644 index 0000000000..1a2c47479f --- /dev/null +++ b/activerecord/test/cases/type/time_test.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/topic" + +module ActiveRecord + module Type + class TimeTest < ActiveRecord::TestCase + def test_default_year_is_correct + expected_time = ::Time.utc(2000, 1, 1, 10, 30, 0) + topic = Topic.new(bonus_time: { 4 => 10, 5 => 30 }) + + assert_equal expected_time, topic.bonus_time + + topic.save! + topic.reload + + assert_equal expected_time, topic.bonus_time + end + end + end +end diff --git a/activerecord/test/cases/unconnected_test.rb b/activerecord/test/cases/unconnected_test.rb index 9eefc32745..f0a0e7f805 100644 --- a/activerecord/test/cases/unconnected_test.rb +++ b/activerecord/test/cases/unconnected_test.rb @@ -29,6 +29,14 @@ class TestUnconnectedAdapter < ActiveRecord::TestCase end end + def test_error_message_when_connection_not_established + error = assert_raise(ActiveRecord::ConnectionNotEstablished) do + TestRecord.find(1) + end + + assert_equal "No connection pool with 'primary' found.", error.message + end + def test_underlying_adapter_no_longer_active assert_not @underlying.active?, "Removed adapter should no longer be active" end diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index 03430154db..a6a47687a2 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -12,17 +12,9 @@ class Topic < ActiveRecord::Base scope :scope_with_lambda, lambda { all } - scope :by_private_lifo, -> { where(author_name: private_lifo) } scope :by_lifo, -> { where(author_name: "lifo") } scope :replied, -> { where "replies_count > 0" } - class << self - private - def private_lifo - "lifo" - end - end - scope "approved_as_string", -> { where(approved: true) } scope :anonymous_extension, -> { } do def one diff --git a/activerecord/test/schema/mysql2_specific_schema.rb b/activerecord/test/schema/mysql2_specific_schema.rb index 61e9bc9af7..b143035213 100644 --- a/activerecord/test/schema/mysql2_specific_schema.rb +++ b/activerecord/test/schema/mysql2_specific_schema.rb @@ -27,6 +27,7 @@ ActiveRecord::Schema.define do create_table :binary_fields, force: true do |t| t.binary :var_binary, limit: 255 t.binary :var_binary_large, limit: 4095 + t.tinyblob :tiny_blob t.blob :normal_blob t.mediumblob :medium_blob @@ -36,6 +37,13 @@ ActiveRecord::Schema.define do t.mediumtext :medium_text t.longtext :long_text + t.binary :tiny_blob_2, size: :tiny + t.binary :medium_blob_2, size: :medium + t.binary :long_blob_2, size: :long + t.text :tiny_text_2, size: :tiny + t.text :medium_text_2, size: :medium + t.text :long_text_2, size: :long + t.index :var_binary end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 7034c773d2..86d5a67a13 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -682,11 +682,7 @@ ActiveRecord::Schema.define do create_table :pets, primary_key: :pet_id, force: true do |t| t.string :name t.integer :owner_id, :integer - if subsecond_precision_supported? - t.timestamps null: false, precision: 6 - else - t.timestamps null: false - end + t.timestamps end create_table :pets_treasures, force: true do |t| @@ -904,11 +900,7 @@ ActiveRecord::Schema.define do t.string :parent_title t.string :type t.string :group - if subsecond_precision_supported? - t.timestamps null: true, precision: 6 - else - t.timestamps null: true - end + t.timestamps null: true end create_table :toys, primary_key: :toy_id, force: true do |t| diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 659bacebde..0c7e0426ae 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,5 +1,11 @@ ## Rails 6.0.0.beta1 (January 18, 2019) ## +* [Rename npm package](https://github.com/rails/rails/pull/34905) from + [`activestorage`](https://www.npmjs.com/package/activestorage) to + [`@rails/activestorage`](https://www.npmjs.com/package/@rails/activestorage). + + *Javan Makhmali* + * Replace `config.active_storage.queue` with two options that indicate which queues analysis and purge jobs should use, respectively: diff --git a/activestorage/README.md b/activestorage/README.md index f658b8d542..2886169ca7 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -101,7 +101,7 @@ Variation of image attachment: ```erb <%# Hitting the variant URL will lazy transform the original blob and then redirect to its new service location %> -<%= image_tag user.avatar.variant(resize_to_fit: [100, 100]) %> +<%= image_tag user.avatar.variant(resize_to_limit: [100, 100]) %> ``` ## Direct uploads diff --git a/activestorage/app/models/active_storage/blob/representable.rb b/activestorage/app/models/active_storage/blob/representable.rb index 03d5511481..32e8fcefdf 100644 --- a/activestorage/app/models/active_storage/blob/representable.rb +++ b/activestorage/app/models/active_storage/blob/representable.rb @@ -10,7 +10,7 @@ module ActiveStorage::Blob::Representable # Returns an ActiveStorage::Variant instance with the set of +transformations+ provided. This is only relevant for image # files, and it allows any image to be transformed for size, colors, and the like. Example: # - # avatar.variant(resize_to_fit: [100, 100]).processed.service_url + # avatar.variant(resize_to_limit: [100, 100]).processed.service_url # # This will create and process a variant of the avatar blob that's constrained to a height and width of 100px. # Then it'll upload said variant to the service according to a derivative key of the blob and the transformations. @@ -18,7 +18,7 @@ module ActiveStorage::Blob::Representable # Frequently, though, you don't actually want to transform the variant right away. But rather simply refer to a # specific variant that can be created by a controller on-demand. Like so: # - # <%= image_tag Current.user.avatar.variant(resize_to_fit: [100, 100]) %> + # <%= image_tag Current.user.avatar.variant(resize_to_limit: [100, 100]) %> # # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::RepresentationsController # can then produce on-demand. @@ -43,13 +43,13 @@ module ActiveStorage::Blob::Representable # from a non-image blob. Active Storage comes with built-in previewers for videos and PDF documents. The video previewer # extracts the first frame from a video and the PDF previewer extracts the first page from a PDF document. # - # blob.preview(resize_to_fit: [100, 100]).processed.service_url + # blob.preview(resize_to_limit: [100, 100]).processed.service_url # # Avoid processing previews synchronously in views. Instead, link to a controller action that processes them on demand. # Active Storage provides one, but you may want to create your own (for example, if you need authentication). Here’s # how to use the built-in version: # - # <%= image_tag video.preview(resize_to_fit: [100, 100]) %> + # <%= image_tag video.preview(resize_to_limit: [100, 100]) %> # # This method raises ActiveStorage::UnpreviewableError if no previewer accepts the receiving blob. To determine # whether a blob is accepted by any previewer, call ActiveStorage::Blob#previewable?. @@ -69,7 +69,7 @@ module ActiveStorage::Blob::Representable # Returns an ActiveStorage::Preview for a previewable blob or an ActiveStorage::Variant for a variable image blob. # - # blob.representation(resize_to_fit: [100, 100]).processed.service_url + # blob.representation(resize_to_limit: [100, 100]).processed.service_url # # Raises ActiveStorage::UnrepresentableError if the receiving blob is neither variable nor previewable. Call # ActiveStorage::Blob#representable? to determine whether a blob is representable. diff --git a/activestorage/app/models/active_storage/preview.rb b/activestorage/app/models/active_storage/preview.rb index dd50494799..bb9d960443 100644 --- a/activestorage/app/models/active_storage/preview.rb +++ b/activestorage/app/models/active_storage/preview.rb @@ -38,7 +38,7 @@ class ActiveStorage::Preview # Processes the preview if it has not been processed yet. Returns the receiving Preview instance for convenience: # - # blob.preview(resize_to_fit: [100, 100]).processed.service_url + # blob.preview(resize_to_limit: [100, 100]).processed.service_url # # Processing a preview generates an image from its blob and attaches the preview image to the blob. Because the preview # image is stored with the blob, it is only generated once. diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index ea57fa5f78..bc0058967a 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -27,7 +27,7 @@ require "ostruct" # To refer to such a delayed on-demand variant, simply link to the variant through the resolved route provided # by Active Storage like so: # -# <%= image_tag Current.user.avatar.variant(resize_to_fit: [100, 100]) %> +# <%= image_tag Current.user.avatar.variant(resize_to_limit: [100, 100]) %> # # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::RepresentationsController # can then produce on-demand. @@ -36,15 +36,15 @@ require "ostruct" # has already been processed and uploaded to the service, and, if so, just return that. Otherwise it will perform # the transformations, upload the variant to the service, and return itself again. Example: # -# avatar.variant(resize_to_fit: [100, 100]).processed.service_url +# avatar.variant(resize_to_limit: [100, 100]).processed.service_url # # This will create and process a variant of the avatar blob that's constrained to a height and width of 100. # Then it'll upload said variant to the service according to a derivative key of the blob and the transformations. # # You can combine any number of ImageMagick/libvips operations into a variant, as well as any macros provided by the -# ImageProcessing gem (such as +resize_to_fit+): +# ImageProcessing gem (such as +resize_to_limit+): # -# avatar.variant(resize_to_fit: [800, 800], monochrome: true, rotate: "-90") +# avatar.variant(resize_to_limit: [800, 800], monochrome: true, rotate: "-90") # # Visit the following links for a list of available ImageProcessing commands and ImageMagick/libvips operations: # diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index 3adc2407e5..67568772da 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -6,7 +6,7 @@ # In case you do need to use this directly, it's instantiated using a hash of transformations where # the key is the command and the value is the arguments. Example: # -# ActiveStorage::Variation.new(resize_to_fit: [100, 100], monochrome: true, trim: true, rotate: "-90") +# ActiveStorage::Variation.new(resize_to_limit: [100, 100], monochrome: true, trim: true, rotate: "-90") # # The options map directly to {ImageProcessing}[https://github.com/janko-m/image_processing] commands. class ActiveStorage::Variation diff --git a/activestorage/lib/active_storage/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb index 18d8ff8237..e56c97f4f5 100644 --- a/activestorage/lib/active_storage/analyzer/video_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb @@ -11,7 +11,7 @@ module ActiveStorage # # Example: # - # ActiveStorage::VideoAnalyzer.new(blob).metadata + # ActiveStorage::Analyzer::VideoAnalyzer.new(blob).metadata # # => { width: 640.0, height: 480.0, duration: 5.0, angle: 0, display_aspect_ratio: [4, 3] } # # When a video's angle is 90 or 270 degrees, its width and height are automatically swapped for convenience. diff --git a/activestorage/test/controllers/blobs_controller_test.rb b/activestorage/test/controllers/blobs_controller_test.rb index 9c811df895..9bf2641de6 100644 --- a/activestorage/test/controllers/blobs_controller_test.rb +++ b/activestorage/test/controllers/blobs_controller_test.rb @@ -20,3 +20,28 @@ class ActiveStorage::BlobsControllerTest < ActionDispatch::IntegrationTest assert_equal "max-age=300, private", @response.headers["Cache-Control"] end end + +if SERVICE_CONFIGURATIONS[:s3] && SERVICE_CONFIGURATIONS[:s3][:access_key_id].present? + class ActiveStorage::S3BlobsControllerTest < ActionDispatch::IntegrationTest + setup do + @old_service = ActiveStorage::Blob.service + ActiveStorage::Blob.service = ActiveStorage::Service.configure(:s3, SERVICE_CONFIGURATIONS) + end + + teardown do + ActiveStorage::Blob.service = @old_service + end + + test "allow redirection to the different host" do + blob = create_file_blob filename: "racecar.jpg" + + assert_nothing_raised { get rails_blob_url(blob) } + assert_response :redirect + assert_no_match @request.host, @response.headers["Location"] + ensure + blob.purge + end + end +else + puts "Skipping S3 redirection tests because no S3 configuration was supplied" +end diff --git a/activestorage/test/controllers/representations_controller_test.rb b/activestorage/test/controllers/representations_controller_test.rb index 2662cc5283..4ae0ff877e 100644 --- a/activestorage/test/controllers/representations_controller_test.rb +++ b/activestorage/test/controllers/representations_controller_test.rb @@ -59,3 +59,33 @@ class ActiveStorage::RepresentationsControllerWithPreviewsTest < ActionDispatch: assert_response :not_found end end + +if SERVICE_CONFIGURATIONS[:s3] && SERVICE_CONFIGURATIONS[:s3][:access_key_id].present? + class ActiveStorage::S3RepresentationsControllerWithVariantsTest < ActionDispatch::IntegrationTest + setup do + @old_service = ActiveStorage::Blob.service + ActiveStorage::Blob.service = ActiveStorage::Service.configure(:s3, SERVICE_CONFIGURATIONS) + end + + teardown do + ActiveStorage::Blob.service = @old_service + end + + test "allow redirection to the different host" do + blob = create_file_blob filename: "racecar.jpg" + + assert_nothing_raised do + get rails_blob_representation_url( + filename: blob.filename, + signed_blob_id: blob.signed_id, + variation_key: ActiveStorage::Variation.encode(resize: "100x100")) + end + assert_response :redirect + assert_no_match @request.host, @response.headers["Location"] + ensure + blob.purge + end + end +else + puts "Skipping S3 redirection tests because no S3 configuration was supplied" +end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 6272dd3471..684ecddb18 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,10 @@ +* Add `ActiveSupport::HashWithIndifferentAccess#assoc`. + + `assoc` can now be called with either a string or a symbol. + + *Stefan Schüßler* + + ## Rails 6.0.0.beta1 (January 18, 2019) ## * Remove deprecated `Module#reachable?` method. diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index f1af76019a..3a2b2652c4 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -164,6 +164,19 @@ module ActiveSupport super(convert_key(key)) end + # Same as <tt>Hash#assoc</tt> where the key passed as argument can be + # either a string or a symbol: + # + # counters = ActiveSupport::HashWithIndifferentAccess.new + # counters[:foo] = 1 + # + # counters.assoc('foo') # => ["foo", 1] + # counters.assoc(:foo) # => ["foo", 1] + # counters.assoc(:zoo) # => nil + def assoc(key) + super(convert_key(key)) + end + # Same as <tt>Hash#fetch</tt> where the key passed as argument can be # either a string or a symbol: # diff --git a/activesupport/test/hash_with_indifferent_access_test.rb b/activesupport/test/hash_with_indifferent_access_test.rb index f81e0dc70f..8a39672609 100644 --- a/activesupport/test/hash_with_indifferent_access_test.rb +++ b/activesupport/test/hash_with_indifferent_access_test.rb @@ -447,6 +447,14 @@ class HashWithIndifferentAccessTest < ActiveSupport::TestCase assert_instance_of ActiveSupport::HashWithIndifferentAccess, indifferent_strings end + def test_indifferent_assoc + indifferent_strings = ActiveSupport::HashWithIndifferentAccess.new(@strings) + key, value = indifferent_strings.assoc(:a) + + assert_equal("a", key) + assert_equal(1, value) + end + def test_indifferent_compact hash_contain_nil_value = @strings.merge("z" => nil) hash = ActiveSupport::HashWithIndifferentAccess.new(hash_contain_nil_value) diff --git a/guides/CHANGELOG.md b/guides/CHANGELOG.md index 5a97fbb586..6432dbfd00 100644 --- a/guides/CHANGELOG.md +++ b/guides/CHANGELOG.md @@ -1,5 +1,13 @@ ## Rails 6.0.0.beta1 (January 18, 2019) ## +* Add "Action Text Overview" Guide. + + *DHH* + +* Add "Action Mailbox Basics" Guide. + + *DHH* + * New section _Troubleshooting_ in the _Autoloading and Reloading Constants_ guide. *Xavier Noria* diff --git a/guides/rails_guides/generator.rb b/guides/rails_guides/generator.rb index fd33c3f8a7..b0ddb0e7e0 100644 --- a/guides/rails_guides/generator.rb +++ b/guides/rails_guides/generator.rb @@ -150,8 +150,8 @@ module RailsGuides puts "Generating #{guide} as #{output_file}" layout = @kindle ? "kindle/layout" : "layout" - view = ActionView::Base.new( - @source_dir, + view = ActionView::Base.with_view_paths( + [@source_dir], edge: @edge, version: @version, mobi: "kindle/#{mobi}", diff --git a/guides/source/active_storage_overview.md b/guides/source/active_storage_overview.md index 474a93c83e..e3bb41ae32 100644 --- a/guides/source/active_storage_overview.md +++ b/guides/source/active_storage_overview.md @@ -434,7 +434,7 @@ original blob into the specified format and redirect to its new service location. ```erb -<%= image_tag user.avatar.variant(resize_to_fit: [100, 100]) %> +<%= image_tag user.avatar.variant(resize_to_limit: [100, 100]) %> ``` To switch to the Vips processor, you would add the following to diff --git a/guides/source/caching_with_rails.md b/guides/source/caching_with_rails.md index 3ac3f8fa8b..56c0ca78a0 100644 --- a/guides/source/caching_with_rails.md +++ b/guides/source/caching_with_rails.md @@ -51,10 +51,10 @@ For instance, it will not impact low-level caching, that we address ### Page Caching Page caching is a Rails mechanism which allows the request for a generated page -to be fulfilled by the webserver (i.e. Apache or NGINX) without having to go +to be fulfilled by the web server (i.e. Apache or NGINX) without having to go through the entire Rails stack. While this is super fast it can't be applied to every situation (such as pages that need authentication). Also, because the -webserver is serving a file directly from the filesystem you will need to +web server is serving a file directly from the filesystem you will need to implement cache expiration. INFO: Page Caching has been removed from Rails 4. See the [actionpack-page_caching gem](https://github.com/rails/actionpack-page_caching). diff --git a/guides/source/configuring.md b/guides/source/configuring.md index a727dcd010..2911b1f7c3 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1239,6 +1239,8 @@ Using Initializer Files After loading the framework and any gems in your application, Rails turns to loading initializers. An initializer is any Ruby file stored under `config/initializers` in your application. You can use initializers to hold configuration settings that should be made after all of the frameworks and gems are loaded, such as options to configure settings for these parts. +NOTE: There is no guarantee that your initializers will run after all the gem initializers, so any initialization code that depends on a given gem having been initialized should go into a `config.after_initialize` block. + NOTE: You can use subfolders to organize your initializers if you like, because Rails will look into the whole file hierarchy from the initializers folder on down. TIP: While Rails supports numbering of initializer file names for load ordering purposes, a better technique is to place any code that need to load in a specific order within the same file. This reduces file name churn, makes dependencies more explicit, and can help surface new concepts within your application. diff --git a/guides/source/development_dependencies_install.md b/guides/source/development_dependencies_install.md index b3baf726e3..e84e5561f2 100644 --- a/guides/source/development_dependencies_install.md +++ b/guides/source/development_dependencies_install.md @@ -227,7 +227,6 @@ If you're using another database, check the file `activerecord/test/config.yml` If you installed Yarn, you will need to install the javascript dependencies: ```bash -$ cd activestorage $ yarn install ``` diff --git a/guides/source/engines.md b/guides/source/engines.md index f15383e3f1..053e3aa16e 100644 --- a/guides/source/engines.md +++ b/guides/source/engines.md @@ -1497,6 +1497,8 @@ To hook into the initialization process of one of the following classes use the | Class | Available Hooks | | --------------------------------- | ------------------------------------ | | `ActionCable` | `action_cable` | +| `ActionCable::Channel::Base` | `action_cable_channel` | +| `ActionCable::Connection::Base` | `action_cable_connection` | | `ActionController::API` | `action_controller_api` | | `ActionController::API` | `action_controller` | | `ActionController::Base` | `action_controller_base` | diff --git a/guides/source/rails_on_rack.md b/guides/source/rails_on_rack.md index c33851a0f9..69b5f254bf 100644 --- a/guides/source/rails_on_rack.md +++ b/guides/source/rails_on_rack.md @@ -35,7 +35,7 @@ application. Any Rack compliant web server should be using ### `rails server` -`rails server` does the basic job of creating a `Rack::Server` object and starting the webserver. +`rails server` does the basic job of creating a `Rack::Server` object and starting the web server. Here's how `rails server` creates an instance of `Rack::Server` diff --git a/guides/source/testing.md b/guides/source/testing.md index 1a2f480407..9667521f3b 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -1775,7 +1775,7 @@ test "connects with cookies" do end ``` -See the API documentation for [`AcionCable::Connection::TestCase`](http://api.rubyonrails.org/classes/ActionCable/Connection/TestCase.html) for more information. +See the API documentation for [`ActionCable::Connection::TestCase`](http://api.rubyonrails.org/classes/ActionCable/Connection/TestCase.html) for more information. ### Channel Test Case @@ -1806,16 +1806,16 @@ require "test_helper" class WebNotificationsChannelTest < ActionCable::Channel::TestCase test "subscribes and stream for user" do - stub_connection current_user: users[:john] + stub_connection current_user: users(:john) subscribe - assert_has_stream_for users[:john] + assert_has_stream_for users(:john) end end ``` -See the API documentation for [`AcionCable::Channel::TestCase`](http://api.rubyonrails.org/classes/ActionCable/Channel/TestCase.html) for more information. +See the API documentation for [`ActionCable::Channel::TestCase`](http://api.rubyonrails.org/classes/ActionCable/Channel/TestCase.html) for more information. ### Custom Assertions And Testing Broadcasts Inside Other Components @@ -1837,6 +1837,33 @@ class ProductTest < ActionCable::TestCase end ``` +If you want to test the broadcasting made with `Channel.broadcast_to`, you shoud use +`Channel.broadcasting_for` to generate an underlying stream name: + +```ruby +# app/jobs/chat_relay_job.rb +class ChatRelayJob < ApplicationJob + def perform_later(room, message) + ChatChannel.broadcast_to room, text: message + end +end + +# test/jobs/chat_relay_job_test.rb +require 'test_helper' + +class ChatRelayJobTest < ActiveJob::TestCase + include ActionCable::TestHelper + + test "broadcast message to room" do + room = rooms(:all) + + assert_broadcast_on(ChatChannel.broadcasting_for(room), text: "Hi!") do + ChatRelayJob.perform_now(room, "Hi!") + end + end +end +``` + Additional Testing Resources ---------------------------- diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 5b249de47e..e55217c5c4 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fix deeply nested namespace command printing. + + *Gannon McGibbon* + + ## Rails 6.0.0.beta1 (January 18, 2019) ## * Remove deprecated `after_bundle` helper inside plugins templates. diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index d5a66b6ec1..b7838f7e32 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -30,7 +30,7 @@ module Rails @filter_parameters = [] @filter_redirect = [] @helpers_paths = [] - @hosts = Array(([IPAddr.new("0.0.0.0/0"), IPAddr.new("::/0"), "localhost"] if Rails.env.development?)) + @hosts = Array(([IPAddr.new("0.0.0.0/0"), IPAddr.new("::/0"), ".localhost"] if Rails.env.development?)) @public_file_server = ActiveSupport::OrderedOptions.new @public_file_server.enabled = true @public_file_server.index_name = "index" diff --git a/railties/lib/rails/command/actions.rb b/railties/lib/rails/command/actions.rb index cbb743346b..50651ad61a 100644 --- a/railties/lib/rails/command/actions.rb +++ b/railties/lib/rails/command/actions.rb @@ -11,10 +11,20 @@ module Rails end def require_application_and_environment! + require_application! + require_environment! + end + + def require_application! require ENGINE_PATH if defined?(ENGINE_PATH) if defined?(APP_PATH) require APP_PATH + end + end + + def require_environment! + if defined?(APP_PATH) Rails.application.require_environment! end end diff --git a/railties/lib/rails/command/base.rb b/railties/lib/rails/command/base.rb index 766872de8a..a22b198c66 100644 --- a/railties/lib/rails/command/base.rb +++ b/railties/lib/rails/command/base.rb @@ -115,7 +115,7 @@ module Rails # For a Rails::Command::TestCommand placed in <tt>rails/command/test_command.rb</tt> # would return <tt>rails/test</tt>. def default_command_root - path = File.expand_path(File.join("../commands", command_root_namespace), __dir__) + path = File.expand_path(relative_command_path, __dir__) path if File.exist?(path) end @@ -135,12 +135,20 @@ module Rails end def command_root_namespace - (namespace.split(":") - %w( rails )).first + (namespace.split(":") - %w(rails)).join(":") + end + + def relative_command_path + File.join("../commands", *command_root_namespace.split(":")) end def namespaced_commands commands.keys.map do |key| - key == command_root_namespace ? key : "#{command_root_namespace}:#{key}" + if command_root_namespace.match?(/(\A|\:)#{key}\z/) + command_root_namespace + else + "#{command_root_namespace}:#{key}" + end end end end diff --git a/railties/lib/rails/commands/credentials/USAGE b/railties/lib/rails/commands/credentials/USAGE index d235592f46..f7268a64d1 100644 --- a/railties/lib/rails/commands/credentials/USAGE +++ b/railties/lib/rails/commands/credentials/USAGE @@ -54,5 +54,5 @@ doesn't exist. The encryption key can also be put in `ENV["RAILS_MASTER_KEY"]`, which takes precedence over the file encryption key. -In addition to that, the default credentials lookup paths can be overriden through +In addition to that, the default credentials lookup paths can be overridden through `config.credentials.content_path` and `config.credentials.key_path`. diff --git a/railties/lib/rails/commands/credentials/credentials_command.rb b/railties/lib/rails/commands/credentials/credentials_command.rb index 852cd401d7..54ccd97506 100644 --- a/railties/lib/rails/commands/credentials/credentials_command.rb +++ b/railties/lib/rails/commands/credentials/credentials_command.rb @@ -20,7 +20,7 @@ module Rails end def edit - require_application_and_environment! + require_application! ensure_editor_available(command: "bin/rails credentials:edit") || (return) @@ -37,7 +37,7 @@ module Rails end def show - require_application_and_environment! + require_application! say credentials.read.presence || missing_credentials_message end diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index f768c30db0..d6c329b581 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -473,9 +473,10 @@ module Rails # files inside eager_load paths. def eager_load! config.eager_load_paths.each do |load_path| - matcher = /\A#{Regexp.escape(load_path.to_s)}\/(.*)\.rb\Z/ + # Starts after load_path plus a slash, ends before ".rb". + relname_range = (load_path.to_s.length + 1)...-3 Dir.glob("#{load_path}/**/*.rb").sort.each do |file| - require_dependency file.sub(matcher, '\1') + require_dependency file[relname_range] end end end diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 0023a9a6a6..0eed552042 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -128,7 +128,7 @@ module Rails def gemfile_entries # :doc: [rails_gemfile_entry, database_gemfile_entry, - webserver_gemfile_entry, + web_server_gemfile_entry, assets_gemfile_entry, webpacker_gemfile_entry, javascript_gemfile_entry, @@ -189,7 +189,7 @@ module Rails "Use #{options[:database]} as the database for Active Record" end - def webserver_gemfile_entry # :doc: + def web_server_gemfile_entry # :doc: return [] if options[:skip_puma] comment = "Use Puma as the app server" GemfileEntry.new("puma", "~> 3.11", comment) 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 08befd9196..94f7dd0c79 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 @@ -98,4 +98,24 @@ Rails.application.configure do # Do not dump schema after migrations. config.active_record.dump_schema_after_migration = false <%- end -%> + + # Inserts middleware to perform automatic connection switching. + # The `database_selector` hash is used to pass options to the DatabaseSelector + # middleware. The `delay` is used to determine how long to wait after a write + # to send a subsequent read to the primary. + # + # The `database_resolver` class is used by the middleware to determine which + # database is appropriate to use based on the time delay. + # + # The `database_operations` class is used by the middleware to set timestamps + # for the last write to the primary. The resolver uses the operations class + # timestamps to determine how long to wait before reading from the replica. + # + # By default Rails will store a last write timestamp in the session. The + # DatabaseSelector middleware is designed as such you can define your own + # strategy for connection switching and pass that into the middleware through + # these configuration options. + # config.active_record.database_selector = { delay: 2.seconds } + # config.active_record.database_resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver + # config.active_record.database_operations = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session end diff --git a/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt index f6146e7259..649253aeca 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt @@ -17,7 +17,7 @@ port ENV.fetch("PORT") { 3000 } environment ENV.fetch("RAILS_ENV") { "development" } # Specifies the number of `workers` to boot in clustered mode. -# Workers are forked webserver processes. If using threads and workers together +# Workers are forked web server processes. If using threads and workers together # the concurrency of the application would be max `threads` * `workers`. # Workers do not work on JRuby or Windows (both of which do not support # processes). diff --git a/railties/lib/rails/tasks/statistics.rake b/railties/lib/rails/tasks/statistics.rake index 8cacf4a49f..5abba7d3b4 100644 --- a/railties/lib/rails/tasks/statistics.rake +++ b/railties/lib/rails/tasks/statistics.rake @@ -9,6 +9,7 @@ STATS_DIRECTORIES = [ %w(Jobs app/jobs), %w(Models app/models), %w(Mailers app/mailers), + %w(Mailboxes app/mailboxes), %w(Channels app/channels), %w(JavaScripts app/assets/javascripts), %w(JavaScript app/javascript), @@ -18,6 +19,8 @@ STATS_DIRECTORIES = [ %w(Helper\ tests test/helpers), %w(Model\ tests test/models), %w(Mailer\ tests test/mailers), + %w(Mailbox\ tests test/mailboxes), + %w(Channel\ tests test/channels), %w(Job\ tests test/jobs), %w(Integration\ tests test/integration), %w(System\ tests test/system), diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 3e979ea20d..9da3956dda 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2289,6 +2289,11 @@ module ApplicationTests MESSAGE end + test "the host whitelist includes .localhost in development" do + app "development" + assert_includes Rails.application.config.hosts, ".localhost" + end + private def force_lazy_load_hooks yield # Tasty clarifying sugar, homie! We only need to reference a constant to load it. diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index 830c36671c..44e3b0f66b 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: 29 Test LOC: 0 Code to Test Ratio: 1:0.0", + assert_match "Code LOC: 32 Test LOC: 0 Code to Test Ratio: 1:0.0", rails("stats") end diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index fda6df500d..f706e53e5c 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -742,6 +742,7 @@ module ApplicationTests def test_reset_sessions_before_rollback_on_system_tests app_file "test/system/reset_session_before_rollback_test.rb", <<-RUBY require "application_system_test_case" + require "selenium/webdriver" class ResetSessionBeforeRollbackTest < ApplicationSystemTestCase def teardown_fixtures @@ -770,6 +771,7 @@ module ApplicationTests def test_reset_sessions_on_failed_system_test_screenshot app_file "test/system/reset_sessions_on_failed_system_test_screenshot_test.rb", <<~RUBY require "application_system_test_case" + require "selenium/webdriver" class ResetSessionsOnFailedSystemTestScreenshotTest < ApplicationSystemTestCase ActionDispatch::SystemTestCase.class_eval do @@ -826,6 +828,7 @@ module ApplicationTests def test_system_tests_are_run_through_rake_test_when_given_in_TEST app_file "test/system/dummy_test.rb", <<-RUBY require "application_system_test_case" + require "selenium/webdriver" class DummyTest < ApplicationSystemTestCase test "something" do diff --git a/railties/test/command/base_test.rb b/railties/test/command/base_test.rb index a49ae8aae7..9132c8b4af 100644 --- a/railties/test/command/base_test.rb +++ b/railties/test/command/base_test.rb @@ -4,10 +4,12 @@ require "abstract_unit" require "rails/command" require "rails/commands/generate/generate_command" require "rails/commands/secrets/secrets_command" +require "rails/commands/db/system/change/change_command" class Rails::Command::BaseTest < ActiveSupport::TestCase test "printing commands" do assert_equal %w(generate), Rails::Command::GenerateCommand.printing_commands assert_equal %w(secrets:setup secrets:edit secrets:show), Rails::Command::SecretsCommand.printing_commands + assert_equal %w(db:system:change), Rails::Command::Db::System::ChangeCommand.printing_commands end end diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb index 7842b0db61..26ffe3070c 100644 --- a/railties/test/commands/credentials_test.rb +++ b/railties/test/commands/credentials_test.rb @@ -63,6 +63,14 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase end end + test "edit command does not raise when an initializer tries to acces non-existent credentials" do + app_file "config/initializers/raise_when_loaded.rb", <<-RUBY + Rails.application.credentials.missing_key! + RUBY + + assert_match(/access_key_id: 123/, run_edit_command(environment: "qa")) + end + test "show credentials" do assert_match(/access_key_id: 123/, run_show_command) end diff --git a/tasks/release.rb b/tasks/release.rb index 6784330fd6..2fdcea9d12 100644 --- a/tasks/release.rb +++ b/tasks/release.rb @@ -218,7 +218,7 @@ namespace :all do <p> <% if @user.avatar.attached? -%> - <%= image_tag @user.avatar.representation(resize_to_fit: [500, 500]) %> + <%= image_tag @user.avatar.representation(resize_to_limit: [500, 500]) %> <% end -%> </p> CODE |