diff options
149 files changed, 1530 insertions, 534 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index dce1a30d9f..618817daca 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -179,6 +179,9 @@ Layout/TrailingWhitespace: Style/UnneededPercentQ: Enabled: true +Lint/ErbNewArguments: + Enabled: true + # Use my_method(my_arg) not my_method( my_arg ) or my_method my_arg. Lint/RequireParentheses: Enabled: true 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 70a29f501d..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) @@ -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) @@ -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/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/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/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 1d2f6b09c3..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`. @@ -154,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/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/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/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..066daa4a12 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 <tt><Driver>::Options</tt> 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 :selenium, using: :chrome, screen_size: [1024, 768] do |driver_option| + # driver_option.add_emulation(device_name: '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/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 998498e1b2..7f1c41787a 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -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 @@ -280,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 @@ -297,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 aadc6be077..6914fb66f9 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -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/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/actiontext/app/models/action_text/rich_text.rb b/actiontext/app/models/action_text/rich_text.rb index 1f39bc51b9..19fa3e030e 100644 --- a/actiontext/app/models/action_text/rich_text.rb +++ b/actiontext/app/models/action_text/rich_text.rb @@ -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/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 16361fd2eb..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* diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index 6cb342a0a9..b143e13c96 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" @@ -187,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 @@ -197,17 +198,51 @@ 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 = nil, assigns = {}, controller = nil, formats = NULL) #:nodoc: @_config = ActiveSupport::InheritableOptions.new - if context.is_a?(ActionView::Renderer) - @view_renderer = context + if formats == NULL + formats = nil + else + 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 = {} 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/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index fb5539e0db..c36baeffcd 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -12,7 +12,7 @@ module ActionView @lookup_context.rendered_format ||= (template.formats.first || formats.first) - render_template(context, template, options[:layout], options[:locals]) + render_template(context, template, options[:layout], options[:locals] || {}) end private @@ -28,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) @@ -36,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." @@ -45,9 +45,7 @@ module ActionView # Renders the given template. A string representing the layout can be # supplied as well. - def render_template(view, template, layout_name = nil, locals = nil) - locals ||= {} - + 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) } @@ -82,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/template/handlers/erb/erubi.rb b/actionview/lib/action_view/template/handlers/erb/erubi.rb index e155bae89d..15ca202024 100644 --- a/actionview/lib/action_view/template/handlers/erb/erubi.rb +++ b/actionview/lib/action_view/template/handlers/erb/erubi.rb @@ -26,7 +26,7 @@ module ActionView 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) - }.new(action_view_erb_handler_context) + }.with_context(action_view_erb_handler_context) view.run(:_template, {}, ActionView::OutputBuffer.new) end 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 d71944e938..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(ActionView::Base.new, {}).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..76ffe3415d 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 @@ -336,6 +336,16 @@ module RenderTestCases assert_equal "Hello: davidHello: mary", @view.render(partial: "test/customer", collection: customers) end + def test_deprecated_constructor + assert_deprecated do + ActionView::Base.new + end + + assert_deprecated do + ActionView::Base.new ["/a"] + end + end + def test_render_partial_without_object_does_not_put_partial_name_to_local_assigns assert_equal "false", @view.render(partial: "test/partial_name_in_local_assigns") end 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/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/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb index d6c80b2c5d..02759b4ccb 100644 --- a/activemodel/lib/active_model/validations/length.rb +++ b/activemodel/lib/active_model/validations/length.rb @@ -32,7 +32,7 @@ module ActiveModel value = options[key] unless (value.is_a?(Integer) && value >= 0) || value == Float::INFINITY || value.is_a?(Symbol) || value.is_a?(Proc) - raise ArgumentError, ":#{key} must be a nonnegative Integer, Infinity, Symbol, or Proc" + raise ArgumentError, ":#{key} must be a non-negative Integer, Infinity, Symbol, or Proc" end end end diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 9cb8b543b0..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: diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index fcdf123062..16c44762cb 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -281,7 +281,7 @@ class NumericalityValidationTest < ActiveModel::TestCase assert_predicate topic, :invalid? end - def test_validates_numericalty_with_object_acting_as_numeric + def test_validates_numericality_with_object_acting_as_numeric klass = Class.new do def to_f 123.54 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..16344b160d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "active_support/deprecation" - module ActiveRecord module ConnectionAdapters #:nodoc: # Abstract representation of an index definition on a table. Instances of @@ -259,10 +257,9 @@ module ActiveRecord include ColumnMethods attr_reader :name, :temporary, :if_not_exists, :options, :as, :comment, :indexes, :foreign_keys - attr_writer :indexes - deprecate :indexes= def initialize( + conn, name, temporary: false, if_not_exists: false, @@ -271,6 +268,7 @@ module ActiveRecord comment: nil, ** ) + @conn = conn @columns_hash = {} @indexes = [] @foreign_keys = [] @@ -410,6 +408,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 90a130320b..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,7 +130,7 @@ 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 } @@ -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_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index d1ff32df3f..9a7d7285f2 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -77,9 +77,6 @@ module ActiveRecord SIMPLE_INT = /\A\d+\z/ - attr_writer :visitor - deprecate :visitor= - attr_accessor :pool attr_reader :schema_cache, :visitor, :owner, :logger, :lock, :prepared_statements, :prevent_writes alias :in_use? :owner 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 5479ddab71..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" }, } @@ -709,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_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 7cf371be68..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) @@ -716,6 +716,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/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..adcfca4f8d --- /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 multiple + # 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..acdb9b3238 --- /dev/null +++ b/activerecord/lib/active_record/middleware/database_selector/resolver.rb @@ -0,0 +1,89 @@ +# 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 inherits 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 + yield + ensure + resolver.update_last_write_timestamp + 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 608182e363..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 @@ -87,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 = {}) @@ -136,7 +155,7 @@ module ActiveRecord class << t prepend TableDefinition end - t + super end end @@ -154,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/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/reflection.rb b/activerecord/lib/active_record/reflection.rb index b2110f727c..6d2f75a3ae 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -612,21 +612,9 @@ module ActiveRecord # returns either +nil+ or the inverse association name that it finds. def automatic_inverse_of - return unless can_find_inverse_of_automatically?(self) + if can_find_inverse_of_automatically?(self) + inverse_name = ActiveSupport::Inflector.underscore(options[:as] || active_record.name.demodulize).to_sym - inverse_name_candidates = - if options[:as] - [options[:as]] - else - active_record_name = active_record.name.demodulize - [active_record_name, ActiveSupport::Inflector.pluralize(active_record_name)] - end - - inverse_name_candidates.map! do |candidate| - ActiveSupport::Inflector.underscore(candidate).to_sym - end - - inverse_name_candidates.detect do |inverse_name| begin reflection = klass._reflect_on_association(inverse_name) rescue NameError @@ -635,7 +623,9 @@ module ActiveRecord reflection = false end - valid_inverse_reflection?(reflection) + if valid_inverse_reflection?(reflection) + return inverse_name + end end end 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 b8f83128c8..1c4b727bf6 100644 --- a/activerecord/lib/arel/nodes/case.rb +++ b/activerecord/lib/arel/nodes/case.rb @@ -2,9 +2,7 @@ module Arel # :nodoc: all module Nodes - class Case < Arel::Nodes::Node - include Arel::AliasPredication - + class Case < Arel::Nodes::NodeExpression attr_accessor :case, :conditions, :default def initialize(expression = nil, default = nil) 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/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index bd535357ee..0133beccec 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -46,6 +46,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase Reader.create person_id: 0, post_id: 0 end + def test_has_many_through_create_record + assert books(:awdr).subscribers.create!(nick: "bob") + end + def test_marshal_dump preloaded = Post.includes(:first_blue_tags).first assert_equal preloaded, Marshal.load(Marshal.dump(preloaded)) diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index eb4dc73423..da3a42e2b5 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -20,8 +20,6 @@ require "models/company" require "models/project" require "models/author" require "models/post" -require "models/department" -require "models/hotel" class AutomaticInverseFindingTests < ActiveRecord::TestCase fixtures :ratings, :comments, :cars @@ -726,16 +724,6 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase # fails because Interest does have the correct inverse_of assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.first.polymorphic_man = Interest.first } end - - def test_favors_has_one_associations_for_inverse_of - inverse_name = Post.reflect_on_association(:author).inverse_of.name - assert_equal :post, inverse_name - end - - def test_finds_inverse_of_for_plural_associations - inverse_name = Department.reflect_on_association(:hotel).inverse_of.name - assert_equal :departments, inverse_name - end end # NOTE - these tests might not be meaningful, ripped as they were from the parental_control plugin 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..6142e223ce --- /dev/null +++ b/activerecord/test/cases/database_selector_test.rb @@ -0,0 +1,114 @@ +# 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_write_to_primary + resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver.new(@session) + + # Session should start empty + assert_nil @session_store[:last_write] + + called = false + resolver.write do + assert ActiveRecord::Base.connected_to?(role: :writing) + called = true + end + assert called + + # and be populated by the last write time + assert @session_store[:last_write] + end + + def test_write_to_primary_with_exception + resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver.new(@session) + + # Session should start empty + assert_nil @session_store[:last_write] + + called = false + assert_raises(ActiveRecord::RecordNotFound) do + resolver.write do + assert ActiveRecord::Base.connected_to?(role: :writing) + called = true + raise ActiveRecord::RecordNotFound + end + end + assert called + + # and be populated by the last write time + assert @session_store[:last_write] + 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/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/subscription.rb b/activerecord/test/models/subscription.rb index d1d5d21621..f87315fcd1 100644 --- a/activerecord/test/models/subscription.rb +++ b/activerecord/test/models/subscription.rb @@ -3,4 +3,6 @@ class Subscription < ActiveRecord::Base belongs_to :subscriber, counter_cache: :books_count belongs_to :book + + validates_presence_of :subscriber_id, :book_id 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/lib/active_storage/previewer/poppler_pdf_previewer.rb b/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb index 69eb617d7b..6bf501a607 100644 --- a/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb +++ b/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb @@ -28,7 +28,7 @@ module ActiveStorage private def draw_first_page_from(file, &block) - # use 72 dpi to match thumbnail dimesions of the PDF + # use 72 dpi to match thumbnail dimensions of the PDF draw self.class.pdftoppm_path, "-singlefile", "-r", "72", "-png", file.path, &block end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 6272dd3471..1e726ceb54 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,14 @@ +* Remove the `` Kernel#` `` override that suppresses ENOENT and accidentally returns nil on Unix systems + + *Akinori Musha* + +* 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/core_ext/kernel.rb b/activesupport/lib/active_support/core_ext/kernel.rb index 0f4356fbdd..7708069301 100644 --- a/activesupport/lib/active_support/core_ext/kernel.rb +++ b/activesupport/lib/active_support/core_ext/kernel.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/kernel/agnostics" require "active_support/core_ext/kernel/concern" require "active_support/core_ext/kernel/reporting" require "active_support/core_ext/kernel/singleton_class" diff --git a/activesupport/lib/active_support/core_ext/kernel/agnostics.rb b/activesupport/lib/active_support/core_ext/kernel/agnostics.rb deleted file mode 100644 index 403b5f31f0..0000000000 --- a/activesupport/lib/active_support/core_ext/kernel/agnostics.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -class Object - # Makes backticks behave (somewhat more) similarly on all platforms. - # On win32 `nonexistent_command` raises Errno::ENOENT; on Unix, the - # spawned shell prints a message to stderr and sets $?. We emulate - # Unix on the former but not the latter. - def `(command) #:nodoc: - super - rescue Errno::ENOENT => e - STDERR.puts "#$0: #{e}" - end -end 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/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/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/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/testing.md b/guides/source/testing.md index f9661c52a0..9667521f3b 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -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/minitest/rails_plugin.rb b/railties/lib/minitest/rails_plugin.rb index 6486fa1798..4b7df6360a 100644 --- a/railties/lib/minitest/rails_plugin.rb +++ b/railties/lib/minitest/rails_plugin.rb @@ -54,6 +54,6 @@ module Minitest end end - # Backwardscompatibility with Rails 5.0 generated plugin test scripts + # Backwards compatibility with Rails 5.0 generated plugin test scripts mattr_reader :run_via, default: {} end diff --git a/railties/lib/rails/app_loader.rb b/railties/lib/rails/app_loader.rb index aabcc5970c..cc057a407d 100644 --- a/railties/lib/rails/app_loader.rb +++ b/railties/lib/rails/app_loader.rb @@ -23,7 +23,7 @@ control: # too that you may or may not want (like yarn) If you already have Rails binstubs in source control, you might be -inadverently overwriting them during deployment by using bundle install +inadvertently overwriting them during deployment by using bundle install with the --binstubs option. If your application was created prior to Rails 4, here's how to upgrade: 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/actions.rb b/railties/lib/rails/generators/actions.rb index 3856a74a39..1a5f2ff203 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -222,6 +222,7 @@ module Rails log :generate, what options = args.extract_options! + options[:without_rails_env] = true argument = args.flat_map(&:to_s).join(" ") execute_command :rails, "generate #{what} #{argument}", options @@ -284,14 +285,15 @@ module Rails # based on the executor parameter provided. def execute_command(executor, command, options = {}) # :doc: log executor, command - env = options[:env] || ENV["RAILS_ENV"] || "development" + env = options[:env] || ENV["RAILS_ENV"] || "development" + rails_env = " RAILS_ENV=#{env}" unless options[:without_rails_env] sudo = options[:sudo] && !Gem.win_platform? ? "sudo " : "" config = { verbose: false } config[:capture] = options[:capture] if options[:capture] config[:abort_on_failure] = options[:abort_on_failure] if options[:abort_on_failure] - in_root { run("#{sudo}#{extify(executor)} #{command} RAILS_ENV=#{env}", config) } + in_root { run("#{sudo}#{extify(executor)} #{command}#{rails_env}", config) } end # Add an extension to the given name based on the platform. diff --git a/railties/lib/rails/generators/migration.rb b/railties/lib/rails/generators/migration.rb index 5081060895..b6ec0160cf 100644 --- a/railties/lib/rails/generators/migration.rb +++ b/railties/lib/rails/generators/migration.rb @@ -63,8 +63,7 @@ module Rails numbered_destination = File.join(dir, ["%migration_number%", base].join("_")) create_migration numbered_destination, nil, config do - match = ERB.version.match(/\Aerb\.rb \[(?<version>[^ ]+) /) - if match && match[:version] >= "2.2.0" # Ruby 2.6+ + if ERB.instance_method(:initialize).parameters.assoc(:key) # Ruby 2.6+ ERB.new(::File.binread(source), trim_mode: "-", eoutvar: "@output_buffer").result(context) else ERB.new(::File.binread(source), nil, "-", "@output_buffer").result(context) 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/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index fda6df500d..0bdd2b314d 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -16,7 +16,7 @@ module ApplicationTests teardown_app end - def test_run_via_backwardscompatibility + def test_run_via_backwards_compatibility require "minitest/rails_plugin" assert_nothing_raised do @@ -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..10d0ba1cae 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 access 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/railties/test/commands/dbconsole_test.rb b/railties/test/commands/dbconsole_test.rb index adb168f7a3..65f6916acb 100644 --- a/railties/test/commands/dbconsole_test.rb +++ b/railties/test/commands/dbconsole_test.rb @@ -112,9 +112,9 @@ class Rails::DBConsoleTest < ActiveSupport::TestCase end def test_mysql_full - start(adapter: "mysql2", database: "db", host: "locahost", port: 1234, socket: "socket", username: "user", password: "qwerty", encoding: "UTF-8") + start(adapter: "mysql2", database: "db", host: "localhost", port: 1234, socket: "socket", username: "user", password: "qwerty", encoding: "UTF-8") assert_not aborted - assert_equal [%w[mysql mysql5], "--host=locahost", "--port=1234", "--socket=socket", "--user=user", "--default-character-set=UTF-8", "-p", "db"], dbconsole.find_cmd_and_exec_args + assert_equal [%w[mysql mysql5], "--host=localhost", "--port=1234", "--socket=socket", "--user=user", "--default-character-set=UTF-8", "-p", "db"], dbconsole.find_cmd_and_exec_args end def test_mysql_include_password diff --git a/railties/test/generators/actions_test.rb b/railties/test/generators/actions_test.rb index 4932100ea2..44d4e92256 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -320,6 +320,12 @@ class ActionsTest < Rails::Generators::TestCase assert_no_file "app/models/my_model.rb" end + def test_generate_should_run_command_without_env + assert_called_with(generator, :run, ["rails generate model MyModel name:string", verbose: false]) do + action :generate, "model", "MyModel", "name:string" + end + end + def test_rake_should_run_rake_command_with_default_env assert_called_with(generator, :run, ["rake log:clear RAILS_ENV=development", verbose: false]) do with_rails_env nil do diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index e0cd7f90ce..860e2bf63e 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -199,7 +199,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def test_new_application_use_json_serialzier + def test_new_application_use_json_serializer run_generator assert_file("config/initializers/cookies_serializer.rb", /Rails\.application\.config\.action_dispatch\.cookies_serializer = :json/) diff --git a/railties/test/generators/generators_test_helper.rb b/railties/test/generators/generators_test_helper.rb index 975a204af4..8b42cb83db 100644 --- a/railties/test/generators/generators_test_helper.rb +++ b/railties/test/generators/generators_test_helper.rb @@ -79,7 +79,11 @@ module GeneratorsTestHelper end def evaluate_template(file, locals = {}) - erb = ERB.new(File.read(file), nil, "-", "@output_buffer") + erb = if ERB.instance_method(:initialize).parameters.assoc(:key) # Ruby 2.6+ + ERB.new(File.read(file), trim_mode: "-", eoutvar: "@output_buffer") + else + ERB.new(File.read(file), nil, "-", "@output_buffer") + end context = Class.new do locals.each do |local, value| class_attribute local, default: value diff --git a/railties/test/generators/mailer_generator_test.rb b/railties/test/generators/mailer_generator_test.rb index ddac6e1a1e..099e06c4d3 100644 --- a/railties/test/generators/mailer_generator_test.rb +++ b/railties/test/generators/mailer_generator_test.rb @@ -119,7 +119,7 @@ class MailerGeneratorTest < Rails::Generators::TestCase assert_match(/haml \[not found\]/, content) end - def test_mailer_with_namedspaced_mailer + def test_mailer_with_namespaced_mailer run_generator ["Farm::Animal", "moos"] assert_file "app/mailers/farm/animal_mailer.rb" do |mailer| assert_match(/class Farm::AnimalMailer < ApplicationMailer/, mailer) diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index b06db6dd8a..134659f285 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -104,7 +104,7 @@ class ModelGeneratorTest < Rails::Generators::TestCase ActiveRecord::Base.pluralize_table_names = true end - def test_migration_with_namespaces_in_model_name_without_plurization + def test_migration_with_namespaces_in_model_name_without_pluralization ActiveRecord::Base.pluralize_table_names = false run_generator ["Gallery::Image"] assert_migration "db/migrate/create_gallery_image", /class CreateGalleryImage < ActiveRecord::Migration\[[0-9.]+\]/ diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 4ec656d18b..f45464f8d0 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -442,7 +442,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase end end - def test_dummy_appplication_skip_listen_by_default + def test_dummy_application_skip_listen_by_default run_generator assert_file "test/dummy/config/environments/development.rb" do |contents| diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 01b6cb0a43..e9cf60a13d 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -483,7 +483,7 @@ Module.new do Dir.chdir(app_template_path) { `yarn add https://github.com/rails/webpacker.git` } # Use the latest version. - # Manually install `webpack` as bin symlinks are not created for subdependencies + # Manually install `webpack` as bin symlinks are not created for sub dependencies # in workspaces. See https://github.com/yarnpkg/yarn/issues/4964 Dir.chdir(app_template_path) { `yarn add webpack@4.17.1 --tilde` } Dir.chdir(app_template_path) { `yarn add webpack-cli` } diff --git a/railties/test/railties/mounted_engine_test.rb b/railties/test/railties/mounted_engine_test.rb index 48f0fbc80f..9755823e51 100644 --- a/railties/test/railties/mounted_engine_test.rb +++ b/railties/test/railties/mounted_engine_test.rb @@ -232,7 +232,7 @@ module ApplicationTests get "/someone/blog/generate_application_route" assert_equal "/", last_response.body - get "/somone/blog/application_route_in_view" + get "/someone/blog/application_route_in_view" assert_equal "/", last_response.body # test generating engine's route from other engine @@ -258,8 +258,8 @@ module ApplicationTests assert_equal "http://example.org/anonymous/blog/posts/44", last_response.body # test that correct path is generated for the same polymorphic_path call in an engine - get "/somone/blog/engine_polymorphic_path" - assert_equal "/somone/blog/posts/44", last_response.body + get "/someone/blog/engine_polymorphic_path" + assert_equal "/someone/blog/posts/44", last_response.body # and in an application get "/application_polymorphic_path" diff --git a/tasks/release.rb b/tasks/release.rb index 6784330fd6..82d1fb6a68 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 @@ -314,8 +314,7 @@ task :announce do require "erb" template = File.read("../tasks/release_announcement_draft.erb") - match = ERB.version.match(/\Aerb\.rb \[(?<version>[^ ]+) /) - if match && match[:version] >= "2.2.0" # Ruby 2.6+ + if ERB.instance_method(:initialize).parameters.assoc(:key) # Ruby 2.6+ puts ERB.new(template, trim_mode: "<>").result(binding) else puts ERB.new(template, nil, "<>").result(binding) |