diff options
120 files changed, 1477 insertions, 850 deletions
diff --git a/.travis.yml b/.travis.yml index 605d1ff247..c648bd2ca7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,7 +21,7 @@ env: - "GEM=aj:integration" - "GEM=guides" rvm: - - 2.2.2 + - 2.2.3 - ruby-head matrix: allow_failures: diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000000..078d5f1219 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,12 @@ +# Contributor Code of Conduct + +The Rails team is committed to fostering a welcoming community. + +**Our Code of Conduct can be found here**: + +http://rubyonrails.org/conduct/ + +For a history of updates, see the page history here: + +https://github.com/rails/rails.github.com/commits/master/conduct/index.html + @@ -7,6 +7,7 @@ gem 'rake', '>= 10.3' # Active Job depends on the URI::GID::MissingModelIDError, which isn't released yet. gem 'globalid', github: 'rails/globalid' +gem 'rack', github: 'rack/rack' # This needs to be with require false as it is # loaded after loading the test library to @@ -20,8 +21,9 @@ gem 'turbolinks' gem 'arel', github: 'rails/arel', branch: 'master' gem 'mail', github: 'mikel/mail' -gem 'sprockets', '~> 3.0.0.rc.1' +gem 'sprockets', github: 'rails/sprockets', branch: 'master' gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'master' +gem 'sass-rails', github: 'rails/sass-rails', branch: 'master' # require: false so bcrypt is loaded only when has_secure_password is used. # This is to avoid ActiveModel (and by extension the entire framework) @@ -31,6 +33,7 @@ gem 'bcrypt', '~> 3.1.10', require: false # This needs to be with require false to avoid # it being automatically loaded by sprockets gem 'uglifier', '>= 1.3.0', require: false +gem 'sass', '>= 3.3', require: false group :doc do gem 'sdoc', '~> 0.4.0' diff --git a/Gemfile.lock b/Gemfile.lock index 52e221c6fc..7cd4832b3a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -26,6 +26,13 @@ GIT mime-types (>= 1.16, < 3) GIT + remote: git://github.com/rack/rack.git + revision: c94e22401d4719b4d78378c7b63362cd692f9005 + specs: + rack (2.0.0.alpha) + json + +GIT remote: git://github.com/rails/arel.git revision: d5432b4616ff43fbb14540d351eed351e21bb20e branch: master @@ -50,15 +57,34 @@ GIT thor (>= 0.14, < 2.0) GIT + remote: git://github.com/rails/sass-rails.git + revision: 805cb17722b8a13ff00dffe20283a6ba2c9a45dc + branch: master + specs: + sass-rails (6.0.0) + railties (>= 4.0.0, < 5.0) + sass (~> 3.3) + sprockets (>= 4.0) + sprockets-rails (< 4.0) + +GIT remote: git://github.com/rails/sprockets-rails.git - revision: ad4a43bd1bb19c86a8bf94a2ad5e477686161490 + revision: e65e088575be4f6b994823b80a277affb0a57a5e branch: master specs: - sprockets-rails (3.0.0) + sprockets-rails (3.0.0.beta2) actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) +GIT + remote: git://github.com/rails/sprockets.git + revision: a61550db7184fc83964fb983547ff09da9efa9be + branch: master + specs: + sprockets (4.0.0) + rack (~> 2.x) + PATH remote: . specs: @@ -71,7 +97,7 @@ PATH actionpack (5.0.0.alpha) actionview (= 5.0.0.alpha) activesupport (= 5.0.0.alpha) - rack (~> 1.6) + rack (~> 2.x) rack-test (~> 0.6.3) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.2) @@ -120,7 +146,7 @@ PATH GEM remote: https://rubygems.org/ specs: - amq-protocol (1.9.2) + amq-protocol (2.0.0) backburner (1.0.0) beaneater (~> 1.0) dante (> 0.1.5) @@ -130,11 +156,10 @@ GEM beaneater (1.0.0) benchmark-ips (2.3.0) builder (3.2.2) - bunny (1.7.0) + bunny (2.0.0) amq-protocol (>= 1.9.2) - byebug (5.0.0) - columnize (= 0.9.0) - celluloid (0.16.1) + byebug (6.0.2) + celluloid (0.16.0) timers (~> 4.0.0) coffee-rails (4.1.0) coffee-script (>= 2.2.0) @@ -143,7 +168,6 @@ GEM coffee-script-source execjs coffee-script-source (1.9.1.1) - columnize (0.9.0) concurrent-ruby (0.9.1) connection_pool (2.2.0) dalli (2.7.4) @@ -154,7 +178,7 @@ GEM activerecord (>= 3.0, < 5.0) delayed_job (>= 3.0, < 4.1) erubis (2.7.0) - execjs (2.5.2) + execjs (2.6.0) hitimes (1.2.2) hitimes (1.2.2-x86-mingw32) i18n (0.7.0) @@ -162,7 +186,7 @@ GEM kindlerb (0.1.1) mustache nokogiri - loofah (2.0.2) + loofah (2.0.3) nokogiri (>= 1.5.9) metaclass (0.0.4) method_source (0.8.2) @@ -179,19 +203,16 @@ GEM nokogiri (1.6.6.2) mini_portile (~> 0.6.0) pg (0.18.2) - psych (2.0.13) + psych (2.0.15) que (0.10.0) racc (1.4.12) - rack (1.6.4) rack-cache (1.2) rack (>= 0.4) - rack-protection (1.5.3) - rack rack-test (0.6.3) rack (>= 1.0) rails-deprecated_sanitizer (1.0.3) activesupport (>= 4.2.0.alpha) - rails-dom-testing (1.0.6) + rails-dom-testing (1.0.7) activesupport (>= 4.2.0.beta, < 5.0) nokogiri (~> 1.6.0) rails-deprecated_sanitizer (>= 1.0.1) @@ -215,6 +236,7 @@ GEM resque (~> 1.25) rufus-scheduler (~> 3.0) rufus-scheduler (3.1.3) + sass (3.4.17) sdoc (0.4.1) json (~> 1.7, >= 1.7.7) rdoc (~> 4.0) @@ -228,25 +250,20 @@ GEM redis (~> 3.2, >= 3.2.1) redis-namespace (~> 1.5, >= 1.5.2) sigdump (0.2.3) - sinatra (1.4.6) - rack (~> 1.4) - rack-protection (~> 1.4) - tilt (>= 1.3, < 3) - sneakers (1.0.4) - bunny (~> 1.7.0) + sinatra (1.0) + rack (>= 1.0) + sneakers (1.1.1) + bunny (>= 1.7.0, <= 2.0.0) serverengine (~> 1.5.5) thor thread (~> 0.1.7) - sprockets (3.0.3) - rack (~> 1.0) sqlite3 (1.3.10) stackprof (0.2.7) - sucker_punch (1.5.0) - celluloid (~> 0.16.0) + sucker_punch (1.5.1) + celluloid (= 0.16.0) thor (0.19.1) thread (0.1.7) thread_safe (0.3.5) - tilt (2.0.1) timers (4.0.1) hitimes turbolinks (2.5.3) @@ -297,17 +314,20 @@ DEPENDENCIES que queue_classic! racc (>= 1.4.6) + rack! rack-cache (~> 1.2) rails! rake (>= 10.3) redcarpet (~> 3.2.3) resque resque-scheduler + sass (>= 3.3) + sass-rails! sdoc (~> 0.4.0) sequel sidekiq sneakers - sprockets (~> 3.0.0.rc.1) + sprockets! sprockets-rails! sqlite3 (~> 1.3.6) stackprof @@ -317,4 +337,4 @@ DEPENDENCIES w3c_validators BUNDLED WITH - 1.10.5 + 1.10.6 @@ -76,6 +76,8 @@ and may also be used independently outside Rails. We encourage you to contribute to Ruby on Rails! Please check out the [Contributing to Ruby on Rails guide](http://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html) for guidelines about how to proceed. [Join us!](http://contributors.rubyonrails.org) +Everyone interacting in Rails and its sub-project’s codebases, issue trackers, chat rooms, and mailing lists is expected to follow the Rails [code of conduct](http://rubyonrails.org/conduct/). + ## Code Status [](https://travis-ci.org/rails/rails) diff --git a/actionmailer/test/abstract_unit.rb b/actionmailer/test/abstract_unit.rb index 706249a2f6..85d3629514 100644 --- a/actionmailer/test/abstract_unit.rb +++ b/actionmailer/test/abstract_unit.rb @@ -9,6 +9,7 @@ silence_warnings do end require 'active_support/testing/autorun' +require 'active_support/testing/method_call_assertions' require 'action_mailer' require 'action_mailer/test_case' @@ -40,4 +41,6 @@ def jruby_skip(message = '') skip message if defined?(JRUBY_VERSION) end -require 'mocha/setup' # FIXME: stop using mocha +class ActiveSupport::TestCase + include ActiveSupport::Testing::MethodCallAssertions +end diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index 59c5638f96..6b2e6a531c 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -505,9 +505,10 @@ class BaseTest < ActiveSupport::TestCase end test "calling deliver on the action should deliver the mail object" do - BaseMailer.expects(:deliver_mail).once - mail = BaseMailer.welcome.deliver_now - assert_equal 'The first email on new API!', mail.subject + assert_called(BaseMailer, :deliver_mail) do + mail = BaseMailer.welcome.deliver_now + assert_equal 'The first email on new API!', mail.subject + end end test "calling deliver on the action should increment the deliveries collection if using the test mailer" do @@ -517,9 +518,11 @@ class BaseTest < ActiveSupport::TestCase test "calling deliver, ActionMailer should yield back to mail to let it call :do_delivery on itself" do mail = Mail::Message.new - mail.expects(:do_delivery).once - BaseMailer.expects(:welcome).returns(mail) - BaseMailer.welcome.deliver + assert_called(mail, :do_delivery) do + assert_called(BaseMailer, :welcome, returns: mail) do + BaseMailer.welcome.deliver + end + end end # Rendering @@ -607,8 +610,9 @@ class BaseTest < ActiveSupport::TestCase mail_side_effects do ActionMailer::Base.register_observer(MyObserver) mail = BaseMailer.welcome - MyObserver.expects(:delivered_email).with(mail) - mail.deliver_now + assert_called_with(MyObserver, :delivered_email, [mail]) do + mail.deliver_now + end end end @@ -616,8 +620,9 @@ class BaseTest < ActiveSupport::TestCase mail_side_effects do ActionMailer::Base.register_observer("BaseTest::MyObserver") mail = BaseMailer.welcome - MyObserver.expects(:delivered_email).with(mail) - mail.deliver_now + assert_called_with(MyObserver, :delivered_email, [mail]) do + mail.deliver_now + end end end @@ -625,8 +630,9 @@ class BaseTest < ActiveSupport::TestCase mail_side_effects do ActionMailer::Base.register_observer(:"base_test/my_observer") mail = BaseMailer.welcome - MyObserver.expects(:delivered_email).with(mail) - mail.deliver_now + assert_called_with(MyObserver, :delivered_email, [mail]) do + mail.deliver_now + end end end @@ -634,9 +640,11 @@ class BaseTest < ActiveSupport::TestCase mail_side_effects do ActionMailer::Base.register_observers("BaseTest::MyObserver", MySecondObserver) mail = BaseMailer.welcome - MyObserver.expects(:delivered_email).with(mail) - MySecondObserver.expects(:delivered_email).with(mail) - mail.deliver_now + assert_called_with(MyObserver, :delivered_email, [mail]) do + assert_called_with(MySecondObserver, :delivered_email, [mail]) do + mail.deliver_now + end + end end end @@ -654,8 +662,9 @@ class BaseTest < ActiveSupport::TestCase mail_side_effects do ActionMailer::Base.register_interceptor(MyInterceptor) mail = BaseMailer.welcome - MyInterceptor.expects(:delivering_email).with(mail) - mail.deliver_now + assert_called_with(MyInterceptor, :delivering_email, [mail]) do + mail.deliver_now + end end end @@ -663,8 +672,9 @@ class BaseTest < ActiveSupport::TestCase mail_side_effects do ActionMailer::Base.register_interceptor("BaseTest::MyInterceptor") mail = BaseMailer.welcome - MyInterceptor.expects(:delivering_email).with(mail) - mail.deliver_now + assert_called_with(MyInterceptor, :delivering_email, [mail]) do + mail.deliver_now + end end end @@ -672,8 +682,9 @@ class BaseTest < ActiveSupport::TestCase mail_side_effects do ActionMailer::Base.register_interceptor(:"base_test/my_interceptor") mail = BaseMailer.welcome - MyInterceptor.expects(:delivering_email).with(mail) - mail.deliver_now + assert_called_with(MyInterceptor, :delivering_email, [mail]) do + mail.deliver_now + end end end @@ -681,18 +692,21 @@ class BaseTest < ActiveSupport::TestCase mail_side_effects do ActionMailer::Base.register_interceptors("BaseTest::MyInterceptor", MySecondInterceptor) mail = BaseMailer.welcome - MyInterceptor.expects(:delivering_email).with(mail) - MySecondInterceptor.expects(:delivering_email).with(mail) - mail.deliver_now + assert_called_with(MyInterceptor, :delivering_email, [mail]) do + assert_called_with(MySecondInterceptor, :delivering_email, [mail]) do + mail.deliver_now + end + end end end test "being able to put proc's into the defaults hash and they get evaluated on mail sending" do mail1 = ProcMailer.welcome['X-Proc-Method'] yesterday = 1.day.ago - Time.stubs(:now).returns(yesterday) - mail2 = ProcMailer.welcome['X-Proc-Method'] - assert(mail1.to_s.to_i > mail2.to_s.to_i) + Time.stub(:now, yesterday) do + mail2 = ProcMailer.welcome['X-Proc-Method'] + assert(mail1.to_s.to_i > mail2.to_s.to_i) + end end test 'default values which have to_proc (e.g. symbols) should not be considered procs' do @@ -877,33 +891,50 @@ class BasePreviewInterceptorsTest < ActiveSupport::TestCase test "you can register a preview interceptor to the mail object that gets passed the mail object before previewing" do ActionMailer::Base.register_preview_interceptor(MyInterceptor) mail = BaseMailer.welcome - BaseMailerPreview.any_instance.stubs(:welcome).returns(mail) - MyInterceptor.expects(:previewing_email).with(mail) - BaseMailerPreview.call(:welcome) + stub_any_instance(BaseMailerPreview) do |instance| + instance.stub(:welcome, mail) do + assert_called_with(MyInterceptor, :previewing_email, [mail]) do + BaseMailerPreview.call(:welcome) + end + end + end end test "you can register a preview interceptor using its stringified name to the mail object that gets passed the mail object before previewing" do ActionMailer::Base.register_preview_interceptor("BasePreviewInterceptorsTest::MyInterceptor") mail = BaseMailer.welcome - BaseMailerPreview.any_instance.stubs(:welcome).returns(mail) - MyInterceptor.expects(:previewing_email).with(mail) - BaseMailerPreview.call(:welcome) + stub_any_instance(BaseMailerPreview) do |instance| + instance.stub(:welcome, mail) do + assert_called_with(MyInterceptor, :previewing_email, [mail]) do + BaseMailerPreview.call(:welcome) + end + end + end end test "you can register an interceptor using its symbolized underscored name to the mail object that gets passed the mail object before previewing" do ActionMailer::Base.register_preview_interceptor(:"base_preview_interceptors_test/my_interceptor") mail = BaseMailer.welcome - BaseMailerPreview.any_instance.stubs(:welcome).returns(mail) - MyInterceptor.expects(:previewing_email).with(mail) - BaseMailerPreview.call(:welcome) + stub_any_instance(BaseMailerPreview) do |instance| + instance.stub(:welcome, mail) do + assert_called_with(MyInterceptor, :previewing_email, [mail]) do + BaseMailerPreview.call(:welcome) + end + end + end end test "you can register multiple preview interceptors to the mail object that both get passed the mail object before previewing" do ActionMailer::Base.register_preview_interceptors("BasePreviewInterceptorsTest::MyInterceptor", MySecondInterceptor) mail = BaseMailer.welcome - BaseMailerPreview.any_instance.stubs(:welcome).returns(mail) - MyInterceptor.expects(:previewing_email).with(mail) - MySecondInterceptor.expects(:previewing_email).with(mail) - BaseMailerPreview.call(:welcome) + stub_any_instance(BaseMailerPreview) do |instance| + instance.stub(:welcome, mail) do + assert_called_with(MyInterceptor, :previewing_email, [mail]) do + assert_called_with(MySecondInterceptor, :previewing_email, [mail]) do + BaseMailerPreview.call(:welcome) + end + end + end + end end end diff --git a/actionmailer/test/delivery_methods_test.rb b/actionmailer/test/delivery_methods_test.rb index 78507ce7dc..ceaa4ca83e 100644 --- a/actionmailer/test/delivery_methods_test.rb +++ b/actionmailer/test/delivery_methods_test.rb @@ -102,16 +102,21 @@ class MailDeliveryTest < ActiveSupport::TestCase end test "ActionMailer should be told when Mail gets delivered" do - DeliveryMailer.expects(:deliver_mail).once - DeliveryMailer.welcome.deliver_now + DeliveryMailer.delivery_method = :test + assert_called(DeliveryMailer, :deliver_mail) do + DeliveryMailer.welcome.deliver_now + end end test "delivery method can be customized per instance" do - Mail::SMTP.any_instance.expects(:deliver!) - email = DeliveryMailer.welcome.deliver_now - assert_instance_of Mail::SMTP, email.delivery_method - email = DeliveryMailer.welcome(delivery_method: :test).deliver_now - assert_instance_of Mail::TestMailer, email.delivery_method + stub_any_instance(Mail::SMTP, instance: Mail::SMTP.new({})) do |instance| + assert_called(instance, :deliver!) do + email = DeliveryMailer.welcome.deliver_now + assert_instance_of Mail::SMTP, email.delivery_method + email = DeliveryMailer.welcome(delivery_method: :test).deliver_now + assert_instance_of Mail::TestMailer, email.delivery_method + end + end end test "delivery method can be customized in subclasses not changing the parent" do @@ -176,8 +181,11 @@ class MailDeliveryTest < ActiveSupport::TestCase old_perform_deliveries = DeliveryMailer.perform_deliveries begin DeliveryMailer.perform_deliveries = false - Mail::Message.any_instance.expects(:deliver!).never - DeliveryMailer.welcome.deliver_now + stub_any_instance(Mail::Message) do |instance| + assert_not_called(instance, :deliver!) do + DeliveryMailer.welcome.deliver_now + end + end ensure DeliveryMailer.perform_deliveries = old_perform_deliveries end diff --git a/actionmailer/test/i18n_with_controller_test.rb b/actionmailer/test/i18n_with_controller_test.rb index 5ffde06a80..04e00cf481 100644 --- a/actionmailer/test/i18n_with_controller_test.rb +++ b/actionmailer/test/i18n_with_controller_test.rb @@ -53,12 +53,15 @@ class ActionMailerI18nWithControllerTest < ActionDispatch::IntegrationTest end def test_send_mail - Mail::SMTP.any_instance.expects(:deliver!) - with_translation 'de', email_subject: '[Anmeldung] Willkommen' do - ActiveSupport::Deprecation.silence do - get '/test/send_mail' + stub_any_instance(Mail::SMTP, instance: Mail::SMTP.new({})) do |instance| + assert_called(instance, :deliver!) do + with_translation 'de', email_subject: '[Anmeldung] Willkommen' do + ActiveSupport::Deprecation.silence do + get '/test/send_mail' + end + assert_equal "Mail sent - Subject: [Anmeldung] Willkommen", @response.body + end end - assert_equal "Mail sent - Subject: [Anmeldung] Willkommen", @response.body end end diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 1bba9df969..28d8bc3091 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -21,7 +21,7 @@ Gem::Specification.new do |s| s.add_dependency 'activesupport', version - s.add_dependency 'rack', '~> 1.6' + s.add_dependency 'rack', '~> 2.x' s.add_dependency 'rack-test', '~> 0.6.3' s.add_dependency 'rails-html-sanitizer', '~> 1.0', '>= 1.0.2' s.add_dependency 'rails-dom-testing', '~> 1.0', '>= 1.0.5' diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index d21a778d8d..e5f3cb8e8d 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -136,7 +136,7 @@ module ActionController #:nodoc: # This is the method that defines the application behavior when a request is found to be unverified. def handle_unverified_request request = @controller.request - request.session = NullSessionHash.new(request.env) + request.session = NullSessionHash.new(request) request.env['action_dispatch.request.flash_hash'] = nil request.env['rack.session.options'] = { skip: true } request.cookie_jar = NullCookieJar.build(request, {}) @@ -145,8 +145,8 @@ module ActionController #:nodoc: protected class NullSessionHash < Rack::Session::Abstract::SessionHash #:nodoc: - def initialize(env) - super(nil, env) + def initialize(req) + super(nil, req) @data = {} @loaded = true end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 94f18cefc3..d14483dc72 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -1,4 +1,5 @@ require 'rack/session/abstract/id' +require 'active_support/core_ext/hash/conversions' require 'active_support/core_ext/object/to_query' require 'active_support/core_ext/module/anonymous' require 'active_support/core_ext/hash/keys' @@ -35,11 +36,15 @@ module ActionController end def query_string=(string) - @env[Rack::QUERY_STRING] = string + set_header Rack::QUERY_STRING, string end def request_parameters=(params) - @env["action_dispatch.request.request_parameters"] = params + set_header "action_dispatch.request.request_parameters", params + end + + def content_type=(type) + set_header 'CONTENT_TYPE', type end def assign_parameters(routes, controller_path, action, parameters, generated_path, query_string_keys) @@ -66,10 +71,12 @@ module ActionController end else if ENCODER.should_multipart?(non_path_parameters) - @env['CONTENT_TYPE'] = ENCODER.content_type + self.content_type = ENCODER.content_type data = ENCODER.build_multipart non_path_parameters else - @env['CONTENT_TYPE'] ||= 'application/x-www-form-urlencoded' + get_header('CONTENT_TYPE') do |k| + set_header k, 'application/x-www-form-urlencoded' + end # FIXME: setting `request_parametes` is normally handled by the # params parser middleware, and we should remove this roundtripping @@ -91,8 +98,8 @@ module ActionController end end - @env['CONTENT_LENGTH'] = data.length.to_s - @env['rack.input'] = StringIO.new(data) + set_header 'CONTENT_LENGTH', data.length.to_s + set_header 'rack.input', StringIO.new(data) end @env["PATH_INFO"] ||= generated_path @@ -449,7 +456,7 @@ module ActionController end if body.present? - @request.env['RAW_POST_DATA'] = body + @request.set_header 'RAW_POST_DATA', body end if http_method.present? @@ -471,15 +478,15 @@ module ActionController end self.cookies.update @request.cookies - @request.env['HTTP_COOKIE'] = cookies.to_header - @request.env['action_dispatch.cookies'] = nil + @request.set_header 'HTTP_COOKIE', cookies.to_header + @request.delete_header 'action_dispatch.cookies' @request = TestRequest.new scrub_env!(@request.env), @request.session @response = build_response @response_klass @response.request = @request @controller.recycle! - @request.env['REQUEST_METHOD'] = http_method + @request.set_header 'REQUEST_METHOD', http_method parameters = parameters.symbolize_keys @@ -493,24 +500,28 @@ module ActionController @request.flash.update(flash || {}) if xhr - @request.env['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' - @request.env['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') + @request.set_header 'HTTP_X_REQUESTED_WITH', 'XMLHttpRequest' + @request.get_header('HTTP_ACCEPT') do |k| + @request.set_header k, [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') + end end @controller.request = @request @controller.response = @response - @request.env["SCRIPT_NAME"] ||= @controller.config.relative_url_root + @request.get_header("SCRIPT_NAME") do |k| + @request.set_header k, @controller.config.relative_url_root + end @controller.recycle! @controller.process(action) - @request.env.delete 'HTTP_COOKIE' + @request.delete_header 'HTTP_COOKIE' - if cookies = @request.env['action_dispatch.cookies'] + if @request.have_cookie_jar? unless @response.committed? - cookies.write(@response) - self.cookies.update(cookies.instance_variable_get(:@cookies)) + @request.cookie_jar.write(@response) + self.cookies.update(@request.cookie_jar.instance_variable_get(:@cookies)) end end @response.prepare! @@ -522,8 +533,8 @@ module ActionController end if xhr - @request.env.delete 'HTTP_X_REQUESTED_WITH' - @request.env.delete 'HTTP_ACCEPT' + @request.delete_header 'HTTP_X_REQUESTED_WITH' + @request.delete_header 'HTTP_ACCEPT' end @request.query_string = '' diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 3170389b36..e70e90018c 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -50,13 +50,13 @@ module ActionDispatch protected def parameter_filter - parameter_filter_for @env.fetch("action_dispatch.parameter_filter") { + parameter_filter_for get_header("action_dispatch.parameter_filter") { return NULL_PARAM_FILTER } end def env_filter - user_key = @env.fetch("action_dispatch.parameter_filter") { + user_key = get_header("action_dispatch.parameter_filter") { return NULL_ENV_FILTER } parameter_filter_for(Array(user_key) + ENV_MATCH) diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index bc5410dc38..fbdec6c132 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -30,27 +30,32 @@ module ActionDispatch HTTP_HEADER = /\A[A-Za-z0-9-]+\z/ include Enumerable - attr_reader :env - def initialize(env = {}) # :nodoc: - @env = env + def self.from_hash(hash) + new ActionDispatch::Request.new hash + end + + def initialize(request) # :nodoc: + @req = request end # Returns the value for the given key mapped to @env. def [](key) - @env[env_name(key)] + @req.get_header env_name(key) end # Sets the given value for the key mapped to @env. def []=(key, value) - @env[env_name(key)] = value + @req.set_header env_name(key), value end def key?(key) - @env.key? env_name(key) + @req.has_header? env_name(key) end alias :include? :key? + DEFAULT = Object.new # :nodoc: + # Returns the value for the given key mapped to @env. # # If the key is not found and an optional code block is not provided, @@ -58,18 +63,22 @@ module ActionDispatch # # If the code block is provided, then it will be run and # its result returned. - def fetch(key, *args, &block) - @env.fetch env_name(key), *args, &block + def fetch(key, default = DEFAULT) + @req.get_header(env_name(key)) do + return default unless default == DEFAULT + return yield if block_given? + raise NameError, key + end end def each(&block) - @env.each(&block) + @req.each_header(&block) end # Returns a new Http::Headers instance containing the contents of # <tt>headers_or_env</tt> and the original instance. def merge(headers_or_env) - headers = Http::Headers.new(env.dup) + headers = @req.dup.headers headers.merge!(headers_or_env) headers end @@ -79,11 +88,14 @@ module ActionDispatch # <tt>headers_or_env</tt>. def merge!(headers_or_env) headers_or_env.each do |key, value| - self[env_name(key)] = value + @req.set_header env_name(key), value end end + def env; @req.env.dup; end + private + # Converts a HTTP header name to an environment variable name if it is # not contained within the headers hash. def env_name(key) diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index ff336b7354..e01d5ecc8f 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -15,12 +15,13 @@ module ActionDispatch # For backward compatibility, the post \format is extracted from the # X-Post-Data-Format HTTP header if present. def content_mime_type - @env["action_dispatch.request.content_type"] ||= begin - if @env['CONTENT_TYPE'] =~ /^([^,\;]*)/ + get_header("action_dispatch.request.content_type") do |k| + v = if get_header('CONTENT_TYPE') =~ /^([^,\;]*)/ Mime::Type.lookup($1.strip.downcase) else nil end + set_header k, v end end @@ -30,14 +31,15 @@ module ActionDispatch # Returns the accepted MIME type for the request. def accepts - @env["action_dispatch.request.accepts"] ||= begin - header = @env['HTTP_ACCEPT'].to_s.strip + get_header("action_dispatch.request.accepts") do |k| + header = get_header('HTTP_ACCEPT').to_s.strip - if header.empty? + v = if header.empty? [content_mime_type] else Mime::Type.parse(header) end + set_header k, v end end @@ -52,14 +54,14 @@ module ActionDispatch end def formats - @env["action_dispatch.request.formats"] ||= begin + get_header("action_dispatch.request.formats") do |k| params_readable = begin parameters[:format] rescue ActionController::BadRequest false end - if params_readable + v = if params_readable Array(Mime[parameters[:format]]) elsif use_accept_header && valid_accept_header accepts @@ -68,6 +70,7 @@ module ActionDispatch else [Mime::HTML] end + set_header k, v end end @@ -102,7 +105,7 @@ module ActionDispatch # end def format=(extension) parameters[:format] = extension.to_s - @env["action_dispatch.request.formats"] = [Mime::Type.lookup_by_extension(parameters[:format])] + set_header "action_dispatch.request.formats", [Mime::Type.lookup_by_extension(parameters[:format])] end # Sets the \formats by string extensions. This differs from #format= by allowing you @@ -121,9 +124,9 @@ module ActionDispatch # end def formats=(extensions) parameters[:format] = extensions.first.to_s - @env["action_dispatch.request.formats"] = extensions.collect do |extension| + set_header "action_dispatch.request.formats", extensions.collect { |extension| Mime::Type.lookup_by_extension(extension) - end + } end # Receives an array of mimes and return the first user sent mime that diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 4defb7f858..277207ae6e 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -8,20 +8,23 @@ module ActionDispatch # Returns both GET and POST \parameters in a single hash. def parameters - @env["action_dispatch.request.parameters"] ||= begin - params = begin - request_parameters.merge(query_parameters) - rescue EOFError - query_parameters.dup - end - params.merge!(path_parameters) - end + params = get_header("action_dispatch.request.parameters") + return params if params + + params = begin + request_parameters.merge(query_parameters) + rescue EOFError + query_parameters.dup + end + params.merge!(path_parameters) + set_header("action_dispatch.request.parameters", params) + params end alias :params :parameters def path_parameters=(parameters) #:nodoc: - @env.delete('action_dispatch.request.parameters') - @env[PARAMETERS_KEY] = parameters + delete_header('action_dispatch.request.parameters') + set_header PARAMETERS_KEY, parameters end # Returns a hash with the \parameters used to form the \path of the request. @@ -29,7 +32,7 @@ module ActionDispatch # # {'action' => 'my_action', 'controller' => 'my_controller'} def path_parameters - @env[PARAMETERS_KEY] ||= {} + get_header(PARAMETERS_KEY) || {} end private diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index de28cd0998..4748a54550 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -20,8 +20,6 @@ module ActionDispatch include ActionDispatch::Http::FilterParameters include ActionDispatch::Http::URL - HTTP_X_REQUEST_ID = "HTTP_X_REQUEST_ID".freeze # :nodoc: - autoload :Session, 'action_dispatch/request/session' autoload :Utils, 'action_dispatch/request/utils' @@ -31,17 +29,19 @@ module ActionDispatch PATH_TRANSLATED REMOTE_HOST REMOTE_IDENT REMOTE_USER REMOTE_ADDR SERVER_NAME SERVER_PROTOCOL + ORIGINAL_SCRIPT_NAME HTTP_ACCEPT HTTP_ACCEPT_CHARSET HTTP_ACCEPT_ENCODING HTTP_ACCEPT_LANGUAGE HTTP_CACHE_CONTROL HTTP_FROM HTTP_NEGOTIATE HTTP_PRAGMA HTTP_CLIENT_IP HTTP_X_FORWARDED_FOR HTTP_VERSION + HTTP_X_REQUEST_ID ].freeze ENV_METHODS.each do |env| class_eval <<-METHOD, __FILE__, __LINE__ + 1 def #{env.sub(/^HTTP_/n, '').downcase} # def accept_charset - @env["#{env}".freeze] # @env["HTTP_ACCEPT_CHARSET".freeze] + get_header "#{env}".freeze # get_header "HTTP_ACCEPT_CHARSET".freeze end # end METHOD end @@ -67,8 +67,20 @@ module ActionDispatch end end + def controller_class + check_path_parameters! + params = path_parameters + controller_param = params[:controller].underscore if params.key?(:controller) + params[:action] ||= 'index' + + yield unless controller_param + + const_name = "#{controller_param.camelize}Controller" + ActiveSupport::Dependencies.constantize(const_name) + end + def key?(key) - @env.key?(key) + has_header? key end # List of HTTP request methods from the following RFCs: @@ -109,44 +121,40 @@ module ActionDispatch end def routes # :nodoc: - env["action_dispatch.routes".freeze] + get_header("action_dispatch.routes".freeze) end def routes=(routes) # :nodoc: - env["action_dispatch.routes".freeze] = routes - end - - def original_script_name # :nodoc: - env['ORIGINAL_SCRIPT_NAME'.freeze] + set_header("action_dispatch.routes".freeze, routes) end def engine_script_name(_routes) # :nodoc: - env[_routes.env_key] + get_header(_routes.env_key) end def engine_script_name=(name) # :nodoc: - env[routes.env_key] = name.dup + set_header(routes.env_key, name.dup) end def request_method=(request_method) #:nodoc: if check_method(request_method) - @request_method = env["REQUEST_METHOD"] = request_method + @request_method = set_header("REQUEST_METHOD", request_method) end end def controller_instance # :nodoc: - env['action_controller.instance'.freeze] + get_header('action_controller.instance'.freeze) end def controller_instance=(controller) # :nodoc: - env['action_controller.instance'.freeze] = controller + set_header('action_controller.instance'.freeze, controller) end def show_exceptions? # :nodoc: # We're treating `nil` as "unset", and we want the default setting to be # `true`. This logic should be extracted to `env_config` and calculated # once. - !(env['action_dispatch.show_exceptions'.freeze] == false) + !(get_header('action_dispatch.show_exceptions'.freeze) == false) end # Returns a symbol form of the #request_method @@ -158,7 +166,7 @@ module ActionDispatch # even if it was overridden by middleware. See #request_method for # more information. def method - @method ||= check_method(env["rack.methodoverride.original_method"] || env['REQUEST_METHOD']) + @method ||= check_method(get_header("rack.methodoverride.original_method") || get_header('REQUEST_METHOD')) end # Returns a symbol form of the #method @@ -170,7 +178,7 @@ module ActionDispatch # # request.headers["Content-Type"] # => "text/plain" def headers - @headers ||= Http::Headers.new(@env) + @headers ||= Http::Headers.new(self) end # Returns a +String+ with the last requested path including their params. @@ -181,7 +189,7 @@ module ActionDispatch # # get '/foo?bar' # request.original_fullpath # => '/foo?bar' def original_fullpath - @original_fullpath ||= (env["ORIGINAL_FULLPATH"] || fullpath) + @original_fullpath ||= (get_header("ORIGINAL_FULLPATH") || fullpath) end # Returns the +String+ full path including params of the last URL requested. @@ -220,7 +228,7 @@ module ActionDispatch # (case-insensitive), which may need to be manually added depending on the # choice of JavaScript libraries and frameworks. def xml_http_request? - @env['HTTP_X_REQUESTED_WITH'] =~ /XMLHttpRequest/i + get_header('HTTP_X_REQUESTED_WITH') =~ /XMLHttpRequest/i end alias :xhr? :xml_http_request? @@ -232,11 +240,11 @@ module ActionDispatch # Returns the IP address of client as a +String+, # usually set by the RemoteIp middleware. def remote_ip - @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s + @remote_ip ||= (get_header("action_dispatch.remote_ip") || ip).to_s end def remote_ip=(remote_ip) - @env["action_dispatch.remote_ip".freeze] = remote_ip + set_header "action_dispatch.remote_ip".freeze, remote_ip end ACTION_DISPATCH_REQUEST_ID = "action_dispatch.request_id".freeze # :nodoc: @@ -248,43 +256,39 @@ module ActionDispatch # This unique ID is useful for tracing a request from end-to-end as part of logging or debugging. # This relies on the rack variable set by the ActionDispatch::RequestId middleware. def request_id - env[ACTION_DISPATCH_REQUEST_ID] + get_header ACTION_DISPATCH_REQUEST_ID end def request_id=(id) # :nodoc: - env[ACTION_DISPATCH_REQUEST_ID] = id + set_header ACTION_DISPATCH_REQUEST_ID, id end alias_method :uuid, :request_id - def x_request_id # :nodoc: - @env[HTTP_X_REQUEST_ID] - end - # Returns the lowercase name of the HTTP server software. def server_software - (@env['SERVER_SOFTWARE'] && /^([a-zA-Z]+)/ =~ @env['SERVER_SOFTWARE']) ? $1.downcase : nil + (get_header('SERVER_SOFTWARE') && /^([a-zA-Z]+)/ =~ get_header('SERVER_SOFTWARE')) ? $1.downcase : nil end # Read the request \body. This is useful for web services that need to # work with raw requests directly. def raw_post - unless @env.include? 'RAW_POST_DATA' + unless has_header? 'RAW_POST_DATA' raw_post_body = body - @env['RAW_POST_DATA'] = raw_post_body.read(content_length) + set_header('RAW_POST_DATA', raw_post_body.read(content_length)) raw_post_body.rewind if raw_post_body.respond_to?(:rewind) end - @env['RAW_POST_DATA'] + get_header 'RAW_POST_DATA' end # The request body is an IO input stream. If the RAW_POST_DATA environment # variable is already set, wrap it in a StringIO. def body - if raw_post = @env['RAW_POST_DATA'] + if raw_post = get_header('RAW_POST_DATA') raw_post.force_encoding(Encoding::BINARY) StringIO.new(raw_post) else - @env['rack.input'] + body_stream end end @@ -295,7 +299,7 @@ module ActionDispatch end def body_stream #:nodoc: - @env['rack.input'] + get_header('rack.input') end # TODO This should be broken apart into AD::Request::Session and probably @@ -306,15 +310,15 @@ module ActionDispatch else self.session = {} end - @env['action_dispatch.request.flash_hash'] = nil + set_header('action_dispatch.request.flash_hash', nil) end def session=(session) #:nodoc: - Session.set @env, session + Session.set self, session end def session_options=(options) - Session::Options.set @env, options + Session::Options.set self, options end # Override Rack's GET method to support indifferent access @@ -336,10 +340,10 @@ module ActionDispatch # Returns the authorization header regardless of whether it was specified directly or through one of the # proxy alternatives. def authorization - @env['HTTP_AUTHORIZATION'] || - @env['X-HTTP_AUTHORIZATION'] || - @env['X_HTTP_AUTHORIZATION'] || - @env['REDIRECT_X_HTTP_AUTHORIZATION'] + get_header('HTTP_AUTHORIZATION') || + get_header('X-HTTP_AUTHORIZATION') || + get_header('X_HTTP_AUTHORIZATION') || + get_header('REDIRECT_X_HTTP_AUTHORIZATION') end # True if the request came from localhost, 127.0.0.1. @@ -348,11 +352,11 @@ module ActionDispatch end def request_parameters=(params) - env["action_dispatch.request.request_parameters".freeze] = params + set_header("action_dispatch.request.request_parameters".freeze, params) end def logger - env["action_dispatch.logger".freeze] + get_header("action_dispatch.logger".freeze) end private diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb index cf6542b370..d069bf0205 100644 --- a/actionpack/lib/action_dispatch/journey/nodes/node.rb +++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb @@ -14,15 +14,15 @@ module ActionDispatch end def each(&block) - Visitors::Each.new(block).accept(self) + Visitors::Each::INSTANCE.accept(self, block) end def to_s - Visitors::String.new.accept(self) + Visitors::String::INSTANCE.accept(self, '') end def to_dot - Visitors::Dot.new.accept(self) + Visitors::Dot::INSTANCE.accept(self) end def to_sym @@ -39,10 +39,14 @@ module ActionDispatch def symbol?; false; end def literal?; false; end + def terminal?; false; end + def star?; false; end + def cat?; false; end end class Terminal < Node # :nodoc: alias :symbol :left + def terminal?; true; end end class Literal < Terminal # :nodoc: @@ -69,11 +73,13 @@ module ActionDispatch class Symbol < Terminal # :nodoc: attr_accessor :regexp alias :symbol :regexp + attr_reader :name DEFAULT_EXP = /[^\.\/\?]+/ def initialize(left) super @regexp = DEFAULT_EXP + @name = left.tr '*:'.freeze, ''.freeze end def default_regexp? @@ -92,6 +98,7 @@ module ActionDispatch end class Star < Unary # :nodoc: + def star?; true; end def type; :STAR; end def name @@ -111,6 +118,7 @@ module ActionDispatch end class Cat < Binary # :nodoc: + def cat?; true; end def type; :CAT; end end diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index 24b90e214d..e93970046c 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -5,7 +5,7 @@ module ActionDispatch attr_reader :spec, :requirements, :anchored def self.from_string string - build(string, {}, ["/.?"], true) + build(string, {}, "/.?", true) end def self.build(path, requirements, separators, anchored) @@ -17,7 +17,7 @@ module ActionDispatch def initialize(ast, requirements, separators, anchored) @spec = ast @requirements = requirements - @separators = separators.join + @separators = separators @anchored = anchored @names = nil @@ -32,12 +32,12 @@ module ActionDispatch end def ast - @spec.grep(Nodes::Symbol).each do |node| + @spec.find_all(&:symbol?).each do |node| re = @requirements[node.to_sym] node.regexp = re if re end - @spec.grep(Nodes::Star).each do |node| + @spec.find_all(&:star?).each do |node| node = node.left node.regexp = @requirements[node.to_sym] || /(.+)/ end @@ -59,31 +59,6 @@ module ActionDispatch }.map(&:name).uniq end - class RegexpOffsets < Journey::Visitors::Visitor # :nodoc: - attr_reader :offsets - - def initialize(matchers) - @matchers = matchers - @capture_count = [0] - end - - def visit(node) - super - @capture_count - end - - def visit_SYMBOL(node) - node = node.to_sym - - if @matchers.key?(node) - re = /#{@matchers[node]}|/ - @capture_count.push((re.match('').length - 1) + (@capture_count.last || 0)) - else - @capture_count << (@capture_count.last || 0) - end - end - end - class AnchoredRegexp < Journey::Visitors::Visitor # :nodoc: def initialize(separator, matchers) @separator = separator @@ -193,8 +168,20 @@ module ActionDispatch def offsets return @offsets if @offsets - viz = RegexpOffsets.new(@requirements) - @offsets = viz.accept(spec) + @offsets = [0] + + spec.find_all(&:symbol?).each do |node| + node = node.to_sym + + if @requirements.key?(node) + re = /#{@requirements[node]}|/ + @offsets.push((re.match('').length - 1) + @offsets.last) + else + @offsets << @offsets.last + end + end + + @offsets end end end diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 1ead6c8a82..f5c9abf1cc 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -1,36 +1,81 @@ module ActionDispatch module Journey # :nodoc: class Route # :nodoc: - attr_reader :app, :path, :defaults, :name + attr_reader :app, :path, :defaults, :name, :precedence attr_reader :constraints alias :conditions :constraints - attr_accessor :precedence + module VerbMatchers + VERBS = %w{ DELETE GET HEAD OPTIONS LINK PATCH POST PUT TRACE UNLINK } + VERBS.each do |v| + class_eval <<-eoc + class #{v} + def self.verb; name.split("::").last; end + def self.call(req); req.#{v.downcase}?; end + end + eoc + end + + class Unknown + attr_reader :verb + + def initialize(verb) + @verb = verb + end + + def call(request); @verb === request.request_method; end + end + + class All + def self.call(_); true; end + def self.verb; ''; end + end + + VERB_TO_CLASS = VERBS.each_with_object({ :all => All }) do |verb, hash| + klass = const_get verb + hash[verb] = klass + hash[verb.downcase] = klass + hash[verb.downcase.to_sym] = klass + end + + end + + def self.verb_matcher(verb) + VerbMatchers::VERB_TO_CLASS.fetch(verb) do + VerbMatchers::Unknown.new verb.to_s.dasherize.upcase + end + end + + def self.build(name, app, path, constraints, required_defaults, defaults) + request_method_match = verb_matcher(constraints.delete(:request_method)) + new name, app, path, constraints, required_defaults, defaults, request_method_match, 0 + end ## # +path+ is a path constraint. # +constraints+ is a hash of constraints to be applied to this route. - def initialize(name, app, path, constraints, required_defaults, defaults) + def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match, precedence) @name = name @app = app @path = path + @request_method_match = request_method_match @constraints = constraints @defaults = defaults @required_defaults = nil - @_required_defaults = required_defaults || [] + @_required_defaults = required_defaults @required_parts = nil @parts = nil @decorated_ast = nil - @precedence = 0 + @precedence = precedence @path_formatter = @path.build_formatter end def ast @decorated_ast ||= begin decorated_ast = path.ast - decorated_ast.grep(Nodes::Terminal).each { |n| n.memo = self } + decorated_ast.find_all(&:terminal?).each { |n| n.memo = self } decorated_ast end end @@ -92,7 +137,8 @@ module ActionDispatch end def matches?(request) - constraints.all? do |method, value| + match_verb(request) && + constraints.all? { |method, value| case value when Regexp, String value === request.send(method).to_s @@ -105,7 +151,7 @@ module ActionDispatch else value === request.send(method) end - end + } end def ip @@ -113,11 +159,20 @@ module ActionDispatch end def requires_matching_verb? - constraints[:request_method] + !@request_method_match.all? { |x| x == VerbMatchers::All } end def verb - constraints[:request_method] || // + %r[^#{verbs.join('|')}$] + end + + private + def verbs + @request_method_match.map(&:verb) + end + + def match_verb(request) + @request_method_match.any? { |m| m.call request } end end end diff --git a/actionpack/lib/action_dispatch/journey/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb index 52b4c8b489..537c9b2f5c 100644 --- a/actionpack/lib/action_dispatch/journey/visitors.rb +++ b/actionpack/lib/action_dispatch/journey/visitors.rb @@ -92,6 +92,45 @@ module ActionDispatch end end + class FunctionalVisitor # :nodoc: + DISPATCH_CACHE = {} + + def accept(node, seed) + visit(node, seed) + end + + def visit node, seed + send(DISPATCH_CACHE[node.type], node, seed) + end + + def binary(node, seed) + visit(node.right, visit(node.left, seed)) + end + def visit_CAT(n, seed); binary(n, seed); end + + def nary(node, seed) + node.children.inject(seed) { |s, c| visit(c, s) } + end + def visit_OR(n, seed); nary(n, seed); end + + def unary(node, seed) + visit(node.left, seed) + end + def visit_GROUP(n, seed); unary(n, seed); end + def visit_STAR(n, seed); unary(n, seed); end + + def terminal(node, seed); seed; end + def visit_LITERAL(n, seed); terminal(n, seed); end + def visit_SYMBOL(n, seed); terminal(n, seed); end + def visit_SLASH(n, seed); terminal(n, seed); end + def visit_DOT(n, seed); terminal(n, seed); end + + instance_methods(false).each do |pim| + next unless pim =~ /^visit_(.*)$/ + DISPATCH_CACHE[$1.to_sym] = pim + end + end + class FormatBuilder < Visitor # :nodoc: def accept(node); Journey::Format.new(super); end def terminal(node); [node.left]; end @@ -117,104 +156,110 @@ module ActionDispatch end # Loop through the requirements AST - class Each < Visitor # :nodoc: - attr_reader :block - - def initialize(block) - @block = block - end - - def visit(node) + class Each < FunctionalVisitor # :nodoc: + def visit(node, block) block.call(node) super end + + INSTANCE = new end - class String < Visitor # :nodoc: + class String < FunctionalVisitor # :nodoc: private - def binary(node) - [visit(node.left), visit(node.right)].join + def binary(node, seed) + visit(node.right, visit(node.left, seed)) end - def nary(node) - node.children.map { |c| visit(c) }.join '|' + def nary(node, seed) + last_child = node.children.last + node.children.inject(seed) { |s, c| + string = visit(c, s) + string << "|".freeze unless last_child == c + string + } end - def terminal(node) - node.left + def terminal(node, seed) + seed + node.left end - def visit_GROUP(node) - "(#{visit(node.left)})" + def visit_GROUP(node, seed) + visit(node.left, seed << "(".freeze) << ")".freeze end + + INSTANCE = new end - class Dot < Visitor # :nodoc: + class Dot < FunctionalVisitor # :nodoc: def initialize @nodes = [] @edges = [] end - def accept(node) + def accept(node, seed = [[], []]) super + nodes, edges = seed <<-eodot digraph parse_tree { size="8,5" node [shape = none]; edge [dir = none]; - #{@nodes.join "\n"} - #{@edges.join("\n")} + #{nodes.join "\n"} + #{edges.join("\n")} } eodot end private - def binary(node) - node.children.each do |c| - @edges << "#{node.object_id} -> #{c.object_id};" - end + def binary(node, seed) + seed.last.concat node.children.map { |c| + "#{node.object_id} -> #{c.object_id};" + } super end - def nary(node) - node.children.each do |c| - @edges << "#{node.object_id} -> #{c.object_id};" - end + def nary(node, seed) + seed.last.concat node.children.map { |c| + "#{node.object_id} -> #{c.object_id};" + } super end - def unary(node) - @edges << "#{node.object_id} -> #{node.left.object_id};" + def unary(node, seed) + seed.last << "#{node.object_id} -> #{node.left.object_id};" super end - def visit_GROUP(node) - @nodes << "#{node.object_id} [label=\"()\"];" + def visit_GROUP(node, seed) + seed.first << "#{node.object_id} [label=\"()\"];" super end - def visit_CAT(node) - @nodes << "#{node.object_id} [label=\"○\"];" + def visit_CAT(node, seed) + seed.first << "#{node.object_id} [label=\"○\"];" super end - def visit_STAR(node) - @nodes << "#{node.object_id} [label=\"*\"];" + def visit_STAR(node, seed) + seed.first << "#{node.object_id} [label=\"*\"];" super end - def visit_OR(node) - @nodes << "#{node.object_id} [label=\"|\"];" + def visit_OR(node, seed) + seed.first << "#{node.object_id} [label=\"|\"];" super end - def terminal(node) + def terminal(node, seed) value = node.left - @nodes << "#{node.object_id} [label=\"#{value}\"];" + seed.first << "#{node.object_id} [label=\"#{value}\"];" + seed end + INSTANCE = new end end end diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index cf4f654ed6..e4c5b0bd29 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -8,48 +8,50 @@ require 'active_support/json' module ActionDispatch class Request < Rack::Request def cookie_jar - env['action_dispatch.cookies'.freeze] ||= Cookies::CookieJar.build(self, cookies) + get_header('action_dispatch.cookies'.freeze) do |k| + self.cookie_jar = Cookies::CookieJar.build(self, cookies) + end end # :stopdoc: def have_cookie_jar? - env.key? 'action_dispatch.cookies'.freeze + has_header? 'action_dispatch.cookies'.freeze end def cookie_jar=(jar) - env['action_dispatch.cookies'.freeze] = jar + set_header 'action_dispatch.cookies'.freeze, jar end def key_generator - env[Cookies::GENERATOR_KEY] + get_header Cookies::GENERATOR_KEY end def signed_cookie_salt - env[Cookies::SIGNED_COOKIE_SALT] + get_header Cookies::SIGNED_COOKIE_SALT end def encrypted_cookie_salt - env[Cookies::ENCRYPTED_COOKIE_SALT] + get_header Cookies::ENCRYPTED_COOKIE_SALT end def encrypted_signed_cookie_salt - env[Cookies::ENCRYPTED_SIGNED_COOKIE_SALT] + get_header Cookies::ENCRYPTED_SIGNED_COOKIE_SALT end def secret_token - env[Cookies::SECRET_TOKEN] + get_header Cookies::SECRET_TOKEN end def secret_key_base - env[Cookies::SECRET_KEY_BASE] + get_header Cookies::SECRET_KEY_BASE end def cookies_serializer - env[Cookies::COOKIES_SERIALIZER] + get_header Cookies::COOKIES_SERIALIZER end def cookies_digest - env[Cookies::COOKIES_DIGEST] + get_header Cookies::COOKIES_DIGEST end # :startdoc: end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 226a688fe2..66bb74b9c5 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -55,18 +55,17 @@ module ActionDispatch response rescue Exception => exception raise exception unless request.show_exceptions? - render_exception(env, exception) + render_exception(request, exception) end private - def render_exception(env, exception) - backtrace_cleaner = env['action_dispatch.backtrace_cleaner'] + def render_exception(request, exception) + backtrace_cleaner = request.get_header('action_dispatch.backtrace_cleaner') wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) - log_error(env, wrapper) + log_error(request, wrapper) - if env['action_dispatch.show_detailed_exceptions'] - request = Request.new(env) + if request.get_header('action_dispatch.show_detailed_exceptions') traces = wrapper.traces trace_to_show = 'Application Trace' @@ -108,8 +107,8 @@ module ActionDispatch [status, {'Content-Type' => "#{format}; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] end - def log_error(env, wrapper) - logger = logger(env) + def log_error(request, wrapper) + logger = logger(request) return unless logger exception = wrapper.exception @@ -125,8 +124,8 @@ module ActionDispatch end end - def logger(env) - env['action_dispatch.logger'] || stderr_logger + def logger(request) + request.logger || stderr_logger end def stderr_logger diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 23da169b22..6041f84834 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -6,15 +6,17 @@ module ActionDispatch # read a notice you put there or <tt>flash["notice"] = "hello"</tt> # to put a new one. def flash - @env[Flash::KEY] ||= Flash::FlashHash.from_session_value(session["flash"]) + flash = flash_hash + return flash if flash + self.flash = Flash::FlashHash.from_session_value(session["flash"]) end def flash=(flash) - @env[Flash::KEY] = flash + set_header Flash::KEY, flash end def flash_hash # :nodoc: - @env[Flash::KEY] + get_header Flash::KEY end end @@ -274,7 +276,7 @@ module ActionDispatch req = ActionDispatch::Request.new env @app.call(env) ensure - session = Request::Session.find(env) || {} + session = Request::Session.find(req) || {} flash_hash = req.flash_hash if flash_hash && (flash_hash.present? || session.key?('flash')) diff --git a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb index 7cde76b30e..0f27984550 100644 --- a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb @@ -17,8 +17,8 @@ module ActionDispatch end def call(env) - status = env["PATH_INFO"][1..-1].to_i request = ActionDispatch::Request.new(env) + status = request.path_info[1..-1].to_i content_type = request.formats.first body = { :status => status, :error => Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) } diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index 84df55fd5a..b924df789f 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -36,6 +36,11 @@ module ActionDispatch @default_options.delete(:sidbits) @default_options.delete(:secure_random) end + + private + def make_request(env) + ActionDispatch::Request.new env + end end module StaleSessionCheck @@ -65,8 +70,8 @@ module ActionDispatch end module SessionObject # :nodoc: - def prepare_session(env) - Request::Session.create(self, env, @default_options) + def prepare_session(req) + Request::Session.create(self, req, @default_options) end def loaded_session?(session) @@ -81,8 +86,7 @@ module ActionDispatch private - def set_cookie(env, session_id, cookie) - request = ActionDispatch::Request.new(env) + def set_cookie(request, session_id, cookie) request.cookie_jar[key] = cookie end end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index d8f9614904..e225f356df 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -71,16 +71,16 @@ module ActionDispatch super(app, options.merge!(:cookie_only => true)) end - def destroy_session(env, session_id, options) + def destroy_session(req, session_id, options) new_sid = generate_sid unless options[:drop] # Reset hash and Assign the new session id - env["action_dispatch.request.unsigned_session_cookie"] = new_sid ? { "session_id" => new_sid } : {} + req.set_header("action_dispatch.request.unsigned_session_cookie", new_sid ? { "session_id" => new_sid } : {}) new_sid end - def load_session(env) + def load_session(req) stale_session_check! do - data = unpacked_cookie_data(env) + data = unpacked_cookie_data(req) data = persistent_session_id!(data) [data["session_id"], data] end @@ -88,20 +88,21 @@ module ActionDispatch private - def extract_session_id(env) + def extract_session_id(req) stale_session_check! do - unpacked_cookie_data(env)["session_id"] + unpacked_cookie_data(req)["session_id"] end end - def unpacked_cookie_data(env) - env["action_dispatch.request.unsigned_session_cookie"] ||= begin - stale_session_check! do - if data = get_cookie(env) + def unpacked_cookie_data(req) + req.get_header("action_dispatch.request.unsigned_session_cookie") do |k| + v = stale_session_check! do + if data = get_cookie(req) data.stringify_keys! end data || {} end + req.set_header k, v end end @@ -111,21 +112,20 @@ module ActionDispatch data end - def set_session(env, sid, session_data, options) + def set_session(req, sid, session_data, options) session_data["session_id"] = sid session_data end - def set_cookie(env, session_id, cookie) - cookie_jar(env)[@key] = cookie + def set_cookie(request, session_id, cookie) + cookie_jar(request)[@key] = cookie end - def get_cookie(env) - cookie_jar(env)[@key] + def get_cookie(req) + cookie_jar(req)[@key] end - def cookie_jar(env) - request = ActionDispatch::Request.new(env) + def cookie_jar(request) request.cookie_jar.signed_or_encrypted end end diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 12d8dab8eb..64695f9738 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -31,7 +31,7 @@ module ActionDispatch @app.call(env) rescue Exception => exception if request.show_exceptions? - render_exception(env, exception) + render_exception(request, exception) else raise exception end @@ -39,14 +39,14 @@ module ActionDispatch private - def render_exception(env, exception) - backtrace_cleaner = env['action_dispatch.backtrace_cleaner'] + def render_exception(request, exception) + backtrace_cleaner = request.get_header 'action_dispatch.backtrace_cleaner' wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) status = wrapper.status_code - env["action_dispatch.exception"] = wrapper.exception - env["action_dispatch.original_path"] = env["PATH_INFO"] - env["PATH_INFO"] = "/#{status}" - response = @exceptions_app.call(env) + request.set_header "action_dispatch.exception", wrapper.exception + request.set_header "action_dispatch.original_path", request.path_info + request.path_info = "/#{status}" + response = @exceptions_app.call(request.env) response[1]['X-Cascade'] == 'pass' ? pass_response(status) : response rescue Exception => failsafe_error $stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}" diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index a8a3cd20b9..b946ccb49f 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -4,38 +4,38 @@ module ActionDispatch class Request < Rack::Request # Session is responsible for lazily loading the session from store. class Session # :nodoc: - ENV_SESSION_KEY = Rack::Session::Abstract::ENV_SESSION_KEY # :nodoc: - ENV_SESSION_OPTIONS_KEY = Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY # :nodoc: + ENV_SESSION_KEY = Rack::RACK_SESSION # :nodoc: + ENV_SESSION_OPTIONS_KEY = Rack::RACK_SESSION_OPTIONS # :nodoc: # Singleton object used to determine if an optional param wasn't specified Unspecified = Object.new # Creates a session hash, merging the properties of the previous session if any - def self.create(store, env, default_options) - session_was = find env - session = Request::Session.new(store, env) + def self.create(store, req, default_options) + session_was = find req + session = Request::Session.new(store, req) session.merge! session_was if session_was - set(env, session) - Options.set(env, Request::Session::Options.new(store, default_options)) + set(req, session) + Options.set(req, Request::Session::Options.new(store, default_options)) session end - def self.find(env) - env[ENV_SESSION_KEY] + def self.find(req) + req.get_header ENV_SESSION_KEY end - def self.set(env, session) - env[ENV_SESSION_KEY] = session + def self.set(req, session) + req.set_header ENV_SESSION_KEY, session end class Options #:nodoc: - def self.set(env, options) - env[ENV_SESSION_OPTIONS_KEY] = options + def self.set(req, options) + req.set_header ENV_SESSION_OPTIONS_KEY, options end - def self.find(env) - env[ENV_SESSION_OPTIONS_KEY] + def self.find(req) + req.get_header ENV_SESSION_OPTIONS_KEY end def initialize(by, default_options) @@ -47,9 +47,9 @@ module ActionDispatch @delegate[key] end - def id(env) + def id(req) @delegate.fetch(:id) { - @by.send(:extract_session_id, env) + @by.send(:extract_session_id, req) } end @@ -58,26 +58,26 @@ module ActionDispatch def values_at(*args); @delegate.values_at(*args); end end - def initialize(by, env) + def initialize(by, req) @by = by - @env = env + @req = req @delegate = {} @loaded = false @exists = nil # we haven't checked yet end def id - options.id(@env) + options.id(@req) end def options - Options.find @env + Options.find @req end def destroy clear options = self.options || {} - @by.send(:destroy_session, @env, options.id(@env), options) + @by.send(:destroy_session, @req, options.id(@req), options) # Load the new sid to be written with the response @loaded = false @@ -181,7 +181,7 @@ module ActionDispatch def exists? return @exists unless @exists.nil? - @exists = @by.send(:session_exists?, @env) + @exists = @by.send(:session_exists?, @req) end def loaded? @@ -209,7 +209,7 @@ module ActionDispatch end def load! - id, session = @by.load_session @env + id, session = @by.load_session @req options[:id] = id @delegate.replace(stringify_keys(session)) @loaded = true diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index a9dae77e51..51bafb539f 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -149,9 +149,10 @@ module ActionDispatch path, conditions, required_defaults, - defaults) + defaults, + request_method, + precedence) - route.precedence = precedence route end @@ -170,36 +171,32 @@ module ActionDispatch def build_conditions(current_conditions, request_class) conditions = current_conditions.dup - # Rack-Mount requires that :request_method be a regular expression. - # :request_method represents the HTTP verb that matches this route. - # - # Here we munge values before they get sent on to rack-mount. - unless @via == [:all] - verbs = @via.map { |m| m.to_s.dasherize.upcase } - conditions[:request_method] = %r[^#{verbs.join('|')}$] - end - conditions.keep_if do |k, _| request_class.public_method_defined?(k) end end private :build_conditions - def build_path(ast, requirements, anchor) - pattern = Journey::Path::Pattern.new(ast, requirements, SEPARATORS, anchor) + def request_method + @via.map { |x| Journey::Route.verb_matcher(x) } + end + private :request_method - builder = Journey::GTG::Builder.new ast + JOINED_SEPARATORS = SEPARATORS.join # :nodoc: + + def build_path(ast, requirements, anchor) + pattern = Journey::Path::Pattern.new(ast, requirements, JOINED_SEPARATORS, anchor) # Get all the symbol nodes followed by literals that are not the # dummy node. - symbols = ast.grep(Journey::Nodes::Symbol).find_all { |n| - builder.followpos(n).first.literal? - } + symbols = ast.find_all { |n| + n.cat? && n.left.symbol? && n.right.cat? && n.right.left.literal? + }.map(&:left) # Get all the symbol nodes preceded by literals. - symbols.concat ast.find_all(&:literal?).map { |n| - builder.followpos(n).first - }.find_all(&:symbol?) + symbols.concat ast.find_all { |n| + n.cat? && n.left.literal? && n.right.cat? && n.right.left.symbol? + }.map { |n| n.right.left } symbols.each { |x| x.regexp = /(?:#{Regexp.union(x.regexp, '-')})+/ @@ -638,7 +635,7 @@ module ActionDispatch # Query if the following named route was already defined. def has_named_route?(name) - @set.named_routes.routes[name.to_sym] + @set.named_routes.key? name end private diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 30e308119d..af6f0de556 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -1,6 +1,5 @@ require 'action_dispatch/journey' require 'forwardable' -require 'thread_safe' require 'active_support/concern' require 'active_support/core_ext/object/to_query' require 'active_support/core_ext/hash/slice' @@ -23,49 +22,27 @@ module ActionDispatch class Dispatcher < Routing::Endpoint def initialize(raise_on_name_error) @raise_on_name_error = raise_on_name_error - @controller_class_names = ThreadSafe::Cache.new end def dispatcher?; true; end def serve(req) - req.check_path_parameters! params = req.path_parameters - - prepare_params!(params) - - controller = controller(params, @raise_on_name_error) do + controller = controller_reference(req) do return [404, {'X-Cascade' => 'pass'}, []] end - dispatch(controller, params[:action], req) - end - - def prepare_params!(params) - normalize_controller!(params) - merge_default_action!(params) - end - - # If this is a default_controller (i.e. a controller specified by the user) - # we should raise an error in case it's not found, because it usually means - # a user error. However, if the controller was retrieved through a dynamic - # segment, as in :controller(/:action), we should simply return nil and - # delegate the control back to Rack cascade. Besides, if this is not a default - # controller, it means we should respect the @scope[:module] parameter. - def controller(params, raise_on_name_error=true) - controller_reference params.fetch(:controller) { yield } rescue NameError => e - raise ActionController::RoutingError, e.message, e.backtrace if raise_on_name_error - yield + if @raise_on_name_error + raise ActionController::RoutingError, e.message, e.backtrace + else + return [404, {'X-Cascade' => 'pass'}, []] + end end protected - - attr_reader :controller_class_names - - def controller_reference(controller_param) - const_name = controller_class_names[controller_param] ||= "#{controller_param.camelize}Controller" - ActiveSupport::Dependencies.constantize(const_name) + def controller_reference(req, &block) + req.controller_class(&block) end private @@ -73,14 +50,6 @@ module ActionDispatch def dispatch(controller, action, req) controller.action(action).call(req.env) end - - def normalize_controller!(params) - params[:controller] = params[:controller].underscore if params.key?(:controller) - end - - def merge_default_action!(params) - params[:action] ||= 'index' - end end # A NamedRouteCollection instance is a collection of named routes, and also @@ -89,6 +58,7 @@ module ActionDispatch class NamedRouteCollection include Enumerable attr_reader :routes, :url_helpers_module, :path_helpers_module + private :routes def initialize @routes = {} @@ -759,14 +729,13 @@ module ActionDispatch req.path_parameters = old_params.merge params app = route.app if app.matches?(req) && app.dispatcher? - dispatcher = app.app - - dispatcher.controller(params, false) do + begin + req.controller_class + rescue NameError raise ActionController::RoutingError, "A route matches #{path.inspect}, but references missing controller: #{params[:controller].camelize}Controller" end - dispatcher.prepare_params!(params) - return params + return req.path_parameters end end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 0cdc6d4e77..4dfd4f3f71 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -359,10 +359,10 @@ module ActionDispatch # this modifies the passed request_env directly if headers.present? - Http::Headers.new(request_env).merge!(headers) + Http::Headers.from_hash(request_env).merge!(headers) end if env.present? - Http::Headers.new(request_env).merge!(env) + Http::Headers.from_hash(request_env).merge!(env) end session = Rack::Test::Session.new(_mock_session) diff --git a/actionpack/test/abstract/translation_test.rb b/actionpack/test/abstract/translation_test.rb index 8289252dfc..1435928578 100644 --- a/actionpack/test/abstract/translation_test.rb +++ b/actionpack/test/abstract/translation_test.rb @@ -24,7 +24,6 @@ module AbstractController }, }, }) - @controller.stubs(action_name: :index) end def test_action_controller_base_responds_to_translate @@ -44,25 +43,34 @@ module AbstractController end def test_lazy_lookup - assert_equal 'bar', @controller.t('.foo') + @controller.stub :action_name, :index do + assert_equal 'bar', @controller.t('.foo') + end end def test_lazy_lookup_with_symbol - assert_equal 'bar', @controller.t(:'.foo') + @controller.stub :action_name, :index do + assert_equal 'bar', @controller.t(:'.foo') + end end def test_lazy_lookup_fallback - assert_equal 'no_action_tr', @controller.t(:'.no_action') + @controller.stub :action_name, :index do + assert_equal 'no_action_tr', @controller.t(:'.no_action') + end end def test_default_translation - assert_equal 'bar', @controller.t('one.two') + @controller.stub :action_name, :index do + assert_equal 'bar', @controller.t('one.two') + end end def test_localize time, expected = Time.gm(2000), 'Sat, 01 Jan 2000 00:00:00 +0000' - I18n.stubs(:localize).with(time).returns(expected) - assert_equal expected, @controller.l(time) + I18n.stub :localize, expected do + assert_equal expected, @controller.l(time) + end end end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 60e2cea8a2..7ea4249e7e 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -122,7 +122,7 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase class StubDispatcher < ::ActionDispatch::Routing::RouteSet::Dispatcher protected def controller_reference(controller_param) - controller_param + controller_param.params[:controller] end def dispatch(controller, action, env) @@ -397,6 +397,7 @@ def jruby_skip(message = '') end require 'mocha/setup' # FIXME: stop using mocha +require 'minitest/mock' class ForkingExecutor class Server diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 9acdc29aeb..4a6086cf78 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1,6 +1,5 @@ require 'abstract_unit' require 'controller/fake_models' -require 'pathname' class TestControllerWithExtraEtags < ActionController::Base etag { nil } @@ -295,9 +294,10 @@ class ExpiresInRenderTest < ActionController::TestCase def test_date_header_when_expires_in time = Time.mktime(2011,10,30) - Time.stubs(:now).returns(time) - get :conditional_hello_with_expires_in - assert_equal Time.now.httpdate, @response.headers["Date"] + Time.stub :now, time do + get :conditional_hello_with_expires_in + assert_equal Time.now.httpdate, @response.headers["Date"] + end end end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 868520a219..f3b0ecac10 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -1,5 +1,4 @@ require 'abstract_unit' -require 'digest/sha1' require "active_support/log_subscriber/test_helper" # common controller actions @@ -133,7 +132,6 @@ end module RequestForgeryProtectionTests def setup @token = "cf50faa3fe97702ca1ae" - @controller.stubs(:form_authenticity_token).returns(@token) @controller.stubs(:valid_authenticity_token?).with{ |_, t| t == @token }.returns(true) @controller.stubs(:valid_authenticity_token?).with{ |_, t| t != @token }.returns(false) @old_request_forgery_protection_token = ActionController::Base.request_forgery_protection_token @@ -145,17 +143,21 @@ module RequestForgeryProtectionTests end def test_should_render_form_with_token_tag - assert_not_blocked do - get :index + @controller.stub :form_authenticity_token, @token do + assert_not_blocked do + get :index + end + assert_select 'form>input[name=?][value=?]', 'custom_authenticity_token', @token end - assert_select 'form>input[name=?][value=?]', 'custom_authenticity_token', @token end def test_should_render_button_to_with_token_tag - assert_not_blocked do - get :show_button + @controller.stub :form_authenticity_token, @token do + assert_not_blocked do + get :show_button + end + assert_select 'form>input[name=?][value=?]', 'custom_authenticity_token', @token end - assert_select 'form>input[name=?][value=?]', 'custom_authenticity_token', @token end def test_should_render_form_without_token_tag_if_remote @@ -199,17 +201,21 @@ module RequestForgeryProtectionTests end def test_should_render_form_with_token_tag_if_remote_and_authenticity_token_requested - assert_not_blocked do - get :form_for_remote_with_token + @controller.stub :form_authenticity_token, @token do + assert_not_blocked do + get :form_for_remote_with_token + end + assert_select 'form>input[name=?][value=?]', 'custom_authenticity_token', @token end - assert_select 'form>input[name=?][value=?]', 'custom_authenticity_token', @token end def test_should_render_form_with_token_tag_with_authenticity_token_requested - assert_not_blocked do - get :form_for_with_token + @controller.stub :form_authenticity_token, @token do + assert_not_blocked do + get :form_for_with_token + end + assert_select 'form>input[name=?][value=?]', 'custom_authenticity_token', @token end - assert_select 'form>input[name=?][value=?]', 'custom_authenticity_token', @token end def test_should_allow_get @@ -402,11 +408,12 @@ class RequestForgeryProtectionControllerUsingResetSessionTest < ActionController end test 'should emit a csrf-param meta tag and a csrf-token meta tag' do - @controller.stubs(:form_authenticity_token).returns(@token + '<=?') - get :meta - assert_select 'meta[name=?][content=?]', 'csrf-param', 'custom_authenticity_token' - assert_select 'meta[name=?]', 'csrf-token' - assert_match(/cf50faa3fe97702ca1ae<=\?/, @response.body) + @controller.stub :form_authenticity_token, @token + '<=?' do + get :meta + assert_select 'meta[name=?][content=?]', 'csrf-param', 'custom_authenticity_token' + assert_select 'meta[name=?]', 'csrf-token' + assert_match(/cf50faa3fe97702ca1ae<=\?/, @response.body) + end end end @@ -485,30 +492,36 @@ class FreeCookieControllerTest < ActionController::TestCase def setup @controller = FreeCookieController.new @token = "cf50faa3fe97702ca1ae" - - SecureRandom.stubs(:base64).returns(@token) super end def test_should_not_render_form_with_token_tag - get :index - assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token, false + SecureRandom.stub :base64, @token do + get :index + assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token, false + end end def test_should_not_render_button_to_with_token_tag - get :show_button - assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token, false + SecureRandom.stub :base64, @token do + get :show_button + assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token, false + end end def test_should_allow_all_methods_without_token - [:post, :patch, :put, :delete].each do |method| - assert_nothing_raised { send(method, :index)} + SecureRandom.stub :base64, @token do + [:post, :patch, :put, :delete].each do |method| + assert_nothing_raised { send(method, :index)} + end end end test 'should not emit a csrf-token meta tag' do - get :meta - assert @response.body.blank? + SecureRandom.stub :base64, @token do + get :meta + assert @response.body.blank? + end end end @@ -529,11 +542,11 @@ class CustomAuthenticityParamControllerTest < ActionController::TestCase def test_should_not_warn_if_form_authenticity_param_matches_form_authenticity_token ActionController::Base.logger = @logger - @controller.stubs(:valid_authenticity_token?).returns(:true) - begin - post :index, params: { custom_token_name: 'foobar' } - assert_equal 0, @logger.logged(:warn).size + @controller.stub :valid_authenticity_token?, :true do + post :index, params: { custom_token_name: 'foobar' } + assert_equal 0, @logger.logged(:warn).size + end ensure ActionController::Base.logger = @old_logger end diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 04d6cc1792..dd7c128566 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -505,8 +505,8 @@ class ResourcesTest < ActionController::TestCase routes = @routes.routes routes.each do |route| routes.each do |r| - next if route === r # skip the comparison instance - assert_not_equal [route.conditions, route.path.spec.to_s], [r.conditions, r.path.spec.to_s] + next if route == r # skip the comparison instance + assert_not_equal [route.conditions, route.path.spec.to_s, route.verb], [r.conditions, r.path.spec.to_s, r.verb] end end end diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index a333290ade..feb3e7eab7 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -327,12 +327,6 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal '/stuff', controller.url_for({ :controller => '/stuff', :only_path => true }) end - def test_ignores_leading_slash - rs.clear! - rs.draw { get '/:controller(/:action(/:id))'} - test_default_setup - end - def test_route_with_colon_first rs.draw do get '/:controller/:action/:id', action: 'index', id: nil diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 1c5de983d8..1f073b4124 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -4,6 +4,8 @@ require 'active_support/json/decoding' require 'rails/engine' class TestCaseTest < ActionController::TestCase + def self.fixture_path; end; + class TestController < ActionController::Base def no_op render plain: 'dummy' @@ -849,10 +851,10 @@ XML end def test_fixture_path_is_accessed_from_self_instead_of_active_support_test_case - TestCaseTest.stubs(:fixture_path).returns(FILES_DIR) - - uploaded_file = fixture_file_upload('/mona_lisa.jpg', 'image/png') - assert_equal File.open("#{FILES_DIR}/mona_lisa.jpg", READ_PLAIN).read, uploaded_file.read + TestCaseTest.stub :fixture_path, FILES_DIR do + uploaded_file = fixture_file_upload('/mona_lisa.jpg', 'image/png') + assert_equal File.open("#{FILES_DIR}/mona_lisa.jpg", READ_PLAIN).read, uploaded_file.read + end end def test_test_uploaded_file_with_binary @@ -893,13 +895,13 @@ XML end def test_fixture_file_upload_relative_to_fixture_path - TestCaseTest.stubs(:fixture_path).returns(FILES_DIR) - uploaded_file = fixture_file_upload("mona_lisa.jpg", "image/jpg") - assert_equal File.open("#{FILES_DIR}/mona_lisa.jpg", READ_PLAIN).read, uploaded_file.read + TestCaseTest.stub :fixture_path, FILES_DIR do + uploaded_file = fixture_file_upload("mona_lisa.jpg", "image/jpg") + assert_equal File.open("#{FILES_DIR}/mona_lisa.jpg", READ_PLAIN).read, uploaded_file.read + end end def test_fixture_file_upload_ignores_nil_fixture_path - TestCaseTest.stubs(:fixture_path).returns(nil) uploaded_file = fixture_file_upload("#{FILES_DIR}/mona_lisa.jpg", "image/jpg") assert_equal File.open("#{FILES_DIR}/mona_lisa.jpg", READ_PLAIN).read, uploaded_file.read end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index aca28ae8d1..3454e60697 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -321,10 +321,12 @@ class CookiesTest < ActionController::TestCase end def test_setting_cookie_with_secure_when_always_write_cookie_is_true - ActionDispatch::Cookies::CookieJar.any_instance.stubs(:always_write_cookie).returns(true) + old_cookie, @request.cookie_jar.always_write_cookie = @request.cookie_jar.always_write_cookie, true get :authenticate_with_secure assert_cookie_header "user_name=david; path=/; secure" assert_equal({"user_name" => "david"}, @response.cookies) + ensure + @request.cookie_jar.always_write_cookie = old_cookie end def test_not_setting_cookie_with_secure diff --git a/actionpack/test/dispatch/exception_wrapper_test.rb b/actionpack/test/dispatch/exception_wrapper_test.rb index 3dac582407..f37cce4d45 100644 --- a/actionpack/test/dispatch/exception_wrapper_test.rb +++ b/actionpack/test/dispatch/exception_wrapper_test.rb @@ -17,8 +17,6 @@ module ActionDispatch end setup do - Rails.stubs(:root).returns(Pathname.new('.')) - @cleaner = ActiveSupport::BacktraceCleaner.new @cleaner.add_silencer { |line| line !~ /^lib/ } end diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index e2b38c23bc..79600b654b 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -1,15 +1,19 @@ require "abstract_unit" class HeaderTest < ActiveSupport::TestCase + def make_headers(hash) + ActionDispatch::Http::Headers.new ActionDispatch::Request.new hash + end + setup do - @headers = ActionDispatch::Http::Headers.new( + @headers = make_headers( "CONTENT_TYPE" => "text/plain", "HTTP_REFERER" => "/some/page" ) end test "#new does not normalize the data" do - headers = ActionDispatch::Http::Headers.new( + headers = make_headers( "Content-Type" => "application/json", "HTTP_REFERER" => "/some/page", "Host" => "http://test.com") @@ -108,7 +112,7 @@ class HeaderTest < ActiveSupport::TestCase end test "env variables with . are not modified" do - headers = ActionDispatch::Http::Headers.new + headers = make_headers({}) headers.merge! "rack.input" => "", "rack.request.cookie_hash" => "", "action_dispatch.logger" => "" @@ -119,7 +123,7 @@ class HeaderTest < ActiveSupport::TestCase end test "symbols are treated as strings" do - headers = ActionDispatch::Http::Headers.new + headers = make_headers({}) headers.merge!(:SERVER_NAME => "example.com", "HTTP_REFERER" => "/", :Host => "test.com") @@ -130,7 +134,7 @@ class HeaderTest < ActiveSupport::TestCase test "headers directly modifies the passed environment" do env = {"HTTP_REFERER" => "/"} - headers = ActionDispatch::Http::Headers.new(env) + headers = make_headers(env) headers['Referer'] = "http://example.com/" headers.merge! "CONTENT_TYPE" => "text/plain" assert_equal({"HTTP_REFERER"=>"http://example.com/", diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index eca4ca8e0e..f35ffd8845 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -82,7 +82,7 @@ module ActionDispatch end assert_equal({:omg=>:awesome, :controller=>"posts", :action=>"index"}, fakeset.defaults.first) - assert_equal(/^GET$/, fakeset.conditions.first[:request_method]) + assert_equal(/^GET$/, fakeset.routes.first.verb) end def test_mapping_requirements @@ -99,7 +99,7 @@ module ActionDispatch mapper.scope(via: :put) do mapper.match '/', :to => 'posts#index', :as => :main end - assert_equal(/^PUT$/, fakeset.conditions.first[:request_method]) + assert_equal(/^PUT$/, fakeset.routes.first.verb) end def test_map_slash diff --git a/actionpack/test/dispatch/mount_test.rb b/actionpack/test/dispatch/mount_test.rb index 6a439be2b5..d027f09762 100644 --- a/actionpack/test/dispatch/mount_test.rb +++ b/actionpack/test/dispatch/mount_test.rb @@ -49,7 +49,7 @@ class TestRoutingMount < ActionDispatch::IntegrationTest def test_app_name_is_properly_generated_when_engine_is_mounted_in_resources assert Router.mounted_helpers.method_defined?(:user_fake_mounted_at_resource), "A mounted helper should be defined with a parent's prefix" - assert Router.named_routes.routes[:user_fake_mounted_at_resource], + assert Router.named_routes.key?(:user_fake_mounted_at_resource), "A named route should be defined with a parent's prefix" end diff --git a/actionpack/test/dispatch/request/session_test.rb b/actionpack/test/dispatch/request/session_test.rb index 10fb04e230..410e3194e2 100644 --- a/actionpack/test/dispatch/request/session_test.rb +++ b/actionpack/test/dispatch/request/session_test.rb @@ -4,40 +4,42 @@ require 'action_dispatch/middleware/session/abstract_store' module ActionDispatch class Request class SessionTest < ActiveSupport::TestCase + attr_reader :req + + def setup + @req = ActionDispatch::Request.new({}) + end + def test_create_adds_itself_to_env - env = {} - s = Session.create(store, env, {}) - assert_equal s, env[Rack::Session::Abstract::ENV_SESSION_KEY] + s = Session.create(store, req, {}) + assert_equal s, req.env[Rack::RACK_SESSION] end def test_to_hash - env = {} - s = Session.create(store, env, {}) + s = Session.create(store, req, {}) s['foo'] = 'bar' assert_equal 'bar', s['foo'] assert_equal({'foo' => 'bar'}, s.to_hash) end def test_create_merges_old - env = {} - s = Session.create(store, env, {}) + s = Session.create(store, req, {}) s['foo'] = 'bar' - s1 = Session.create(store, env, {}) + s1 = Session.create(store, req, {}) assert_not_equal s, s1 assert_equal 'bar', s1['foo'] end def test_find - env = {} - assert_nil Session.find(env) + assert_nil Session.find(req) - s = Session.create(store, env, {}) - assert_equal s, Session.find(env) + s = Session.create(store, req, {}) + assert_equal s, Session.find(req) end def test_destroy - s = Session.create(store, {}, {}) + s = Session.create(store, req, {}) s['rails'] = 'ftw' s.destroy @@ -46,21 +48,21 @@ module ActionDispatch end def test_keys - s = Session.create(store, {}, {}) + s = Session.create(store, req, {}) s['rails'] = 'ftw' s['adequate'] = 'awesome' assert_equal %w[rails adequate], s.keys end def test_values - s = Session.create(store, {}, {}) + s = Session.create(store, req, {}) s['rails'] = 'ftw' s['adequate'] = 'awesome' assert_equal %w[ftw awesome], s.values end def test_clear - s = Session.create(store, {}, {}) + s = Session.create(store, req, {}) s['rails'] = 'ftw' s['adequate'] = 'awesome' @@ -69,7 +71,7 @@ module ActionDispatch end def test_update - s = Session.create(store, {}, {}) + s = Session.create(store, req, {}) s['rails'] = 'ftw' s.update(:rails => 'awesome') @@ -79,7 +81,7 @@ module ActionDispatch end def test_delete - s = Session.create(store, {}, {}) + s = Session.create(store, req, {}) s['rails'] = 'ftw' s.delete('rails') @@ -88,7 +90,7 @@ module ActionDispatch end def test_fetch - session = Session.create(store, {}, {}) + session = Session.create(store, req, {}) session['one'] = '1' assert_equal '1', session.fetch(:one) diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 332a550de0..8972f3e74d 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1,4 +1,3 @@ -# encoding: UTF-8 require 'erb' require 'abstract_unit' require 'controller/fake_controllers' @@ -4190,11 +4189,11 @@ class TestNamedRouteUrlHelpers < ActionDispatch::IntegrationTest include Routes.url_helpers test "url helpers do not ignore nil parameters when using non-optimized routes" do - Routes.stubs(:optimize_routes_generation?).returns(false) - - get "/categories/1" - assert_response :success - assert_raises(ActionController::UrlGenerationError) { product_path(nil) } + Routes.stub :optimize_routes_generation?, false do + get "/categories/1" + assert_response :success + assert_raises(ActionController::UrlGenerationError) { product_path(nil) } + end end end diff --git a/actionpack/test/dispatch/session/abstract_store_test.rb b/actionpack/test/dispatch/session/abstract_store_test.rb index fe1a7b4f86..1c35144e6f 100644 --- a/actionpack/test/dispatch/session/abstract_store_test.rb +++ b/actionpack/test/dispatch/session/abstract_store_test.rb @@ -27,7 +27,7 @@ module ActionDispatch as.call(env) assert @env - assert Request::Session.find @env + assert Request::Session.find ActionDispatch::Request.new @env end def test_new_session_object_is_merged_with_old @@ -36,11 +36,11 @@ module ActionDispatch as.call(env) assert @env - session = Request::Session.find @env + session = Request::Session.find ActionDispatch::Request.new @env session['foo'] = 'bar' as.call(@env) - session1 = Request::Session.find @env + session1 = Request::Session.find ActionDispatch::Request.new @env assert_not_equal session, session1 assert_equal session.to_hash, session1.to_hash diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index e432c65c62..f07e215e3a 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -274,28 +274,32 @@ class CookieStoreTest < ActionDispatch::IntegrationTest with_test_route_set(:expire_after => 5.hours) do # First request accesses the session time = Time.local(2008, 4, 24) - Time.stubs(:now).returns(time) - expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") + cookie_body = nil - cookies[SessionKey] = SignedBar + Time.stub :now, time do + expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") - get '/set_session_value' - assert_response :success + cookies[SessionKey] = SignedBar - cookie_body = response.body - assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly", - headers['Set-Cookie'] + get '/set_session_value' + assert_response :success + + cookie_body = response.body + assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly", + headers['Set-Cookie'] + end # Second request does not access the session time = Time.local(2008, 4, 25) - Time.stubs(:now).returns(time) - expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") + Time.stub :now, time do + expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") - get '/no_session_access' - assert_response :success + get '/no_session_access' + assert_response :success - assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly", - headers['Set-Cookie'] + assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly", + headers['Set-Cookie'] + end end end diff --git a/actionpack/test/dispatch/test_request_test.rb b/actionpack/test/dispatch/test_request_test.rb index ede1cec4e6..51c469a61a 100644 --- a/actionpack/test/dispatch/test_request_test.rb +++ b/actionpack/test/dispatch/test_request_test.rb @@ -53,10 +53,8 @@ class TestRequestTest < ActiveSupport::TestCase assert_cookies({"user_name" => "david"}, req.cookie_jar) end - test "does not complain when Rails.application is nil" do - Rails.stubs(:application).returns(nil) + test "does not complain when there is no application config" do req = ActionDispatch::TestRequest.create({}) - assert_equal false, req.env.empty? end diff --git a/actionpack/test/journey/nodes/symbol_test.rb b/actionpack/test/journey/nodes/symbol_test.rb index d411a5018a..adf85b860c 100644 --- a/actionpack/test/journey/nodes/symbol_test.rb +++ b/actionpack/test/journey/nodes/symbol_test.rb @@ -5,7 +5,7 @@ module ActionDispatch module Nodes class TestSymbol < ActiveSupport::TestCase def test_default_regexp? - sym = Symbol.new nil + sym = Symbol.new "foo" assert sym.default_regexp? sym.regexp = nil diff --git a/actionpack/test/journey/path/pattern_test.rb b/actionpack/test/journey/path/pattern_test.rb index 7b97379bd5..72858f5eda 100644 --- a/actionpack/test/journey/path/pattern_test.rb +++ b/actionpack/test/journey/path/pattern_test.rb @@ -4,6 +4,8 @@ module ActionDispatch module Journey module Path class TestPattern < ActiveSupport::TestCase + SEPARATORS = ["/", ".", "?"].join + x = /.+/ { '/:controller(/:action)' => %r{\A/(#{x})(?:/([^/.?]+))?\Z}, @@ -22,7 +24,7 @@ module ActionDispatch path = Pattern.build( path, { :controller => /.+/ }, - ["/", ".", "?"], + SEPARATORS, true ) assert_equal(expected, path.to_regexp) @@ -46,7 +48,7 @@ module ActionDispatch path = Pattern.build( path, { :controller => /.+/ }, - ["/", ".", "?"], + SEPARATORS, false ) assert_equal(expected, path.to_regexp) @@ -69,7 +71,7 @@ module ActionDispatch path = Pattern.build( path, { :controller => /.+/ }, - ["/", ".", "?"], + SEPARATORS, true ) assert_equal(expected, path.names) @@ -84,7 +86,7 @@ module ActionDispatch (tender|love #MAO )/x }, - ["/", ".", "?"], + SEPARATORS, true ) assert_match(path, '/page/tender') @@ -107,7 +109,7 @@ module ActionDispatch path = Pattern.build( '/:name', { :name => /\d+/ }, - ["/", ".", "?"], + SEPARATORS, true ) assert_match(path, '/123') @@ -118,7 +120,7 @@ module ActionDispatch path = Pattern.build( '/page/:name', { :name => /(tender|love)/ }, - ["/", ".", "?"], + SEPARATORS, true ) assert_match(path, '/page/tender') @@ -131,7 +133,7 @@ module ActionDispatch path = Pattern.build( '/page/:name/:value', requirements, - ["/", ".", "?"], + SEPARATORS, true ) @@ -146,7 +148,7 @@ module ActionDispatch path = Pattern.build( '/page/:name', { :name => /(tender|love)/ }, - ["/", ".", "?"], + SEPARATORS, true ) match = path.match '/page/tender' @@ -158,7 +160,7 @@ module ActionDispatch path = Pattern.build( '/page/:name/:id', { :name => /t(((ender|love)))()/ }, - ["/", ".", "?"], + SEPARATORS, true ) match = path.match '/page/tender/10' @@ -173,7 +175,7 @@ module ActionDispatch path = Pattern.build( '/page/*foo', { :foo => z }, - ["/", ".", "?"], + SEPARATORS, true ) assert_equal(%r{\A/page/(#{z})\Z}, path.to_regexp) @@ -183,7 +185,7 @@ module ActionDispatch path = Pattern.build( '/page/:name/aaron', { :name => /(tender|love)/i }, - ["/", ".", "?"], + SEPARATORS, true ) assert_match(path, '/page/TENDER/aaron') @@ -192,7 +194,7 @@ module ActionDispatch end def test_to_regexp_with_strexp - path = Pattern.build('/:controller', { }, ["/", ".", "?"], true) + path = Pattern.build('/:controller', { }, SEPARATORS, true) x = %r{\A/([^/.?]+)\Z} assert_equal(x.source, path.source) diff --git a/actionpack/test/journey/route_test.rb b/actionpack/test/journey/route_test.rb index cdd59a3316..22c3b8113d 100644 --- a/actionpack/test/journey/route_test.rb +++ b/actionpack/test/journey/route_test.rb @@ -7,7 +7,7 @@ module ActionDispatch app = Object.new path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))' defaults = {} - route = Route.new("name", app, path, {}, [], defaults) + route = Route.build("name", app, path, {}, [], defaults) assert_equal app, route.app assert_equal path, route.path @@ -18,7 +18,7 @@ module ActionDispatch app = Object.new path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))' defaults = {} - route = Route.new("name", app, path, {}, [], defaults) + route = Route.build("name", app, path, {}, [], defaults) route.ast.grep(Nodes::Terminal).each do |node| assert_equal route, node.memo @@ -26,29 +26,29 @@ module ActionDispatch end def test_path_requirements_override_defaults - path = Path::Pattern.build(':name', { name: /love/ }, ['/'], true) + path = Path::Pattern.build(':name', { name: /love/ }, '/', true) defaults = { name: 'tender' } - route = Route.new('name', nil, path, nil, [], defaults) + route = Route.build('name', nil, path, {}, [], defaults) assert_equal(/love/, route.requirements[:name]) end def test_ip_address path = Path::Pattern.from_string '/messages/:id(.:format)' - route = Route.new("name", nil, path, {:ip => '192.168.1.1'}, [], + route = Route.build("name", nil, path, {:ip => '192.168.1.1'}, [], { :controller => 'foo', :action => 'bar' }) assert_equal '192.168.1.1', route.ip end def test_default_ip path = Path::Pattern.from_string '/messages/:id(.:format)' - route = Route.new("name", nil, path, {}, [], + route = Route.build("name", nil, path, {}, [], { :controller => 'foo', :action => 'bar' }) assert_equal(//, route.ip) end def test_format_with_star path = Path::Pattern.from_string '/:controller/*extra' - route = Route.new("name", nil, path, {}, [], + route = Route.build("name", nil, path, {}, [], { :controller => 'foo', :action => 'bar' }) assert_equal '/foo/himom', route.format({ :controller => 'foo', @@ -58,7 +58,7 @@ module ActionDispatch def test_connects_all_match path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))' - route = Route.new("name", nil, path, {:action => 'bar'}, [], { :controller => 'foo' }) + route = Route.build("name", nil, path, {:action => 'bar'}, [], { :controller => 'foo' }) assert_equal '/foo/bar/10', route.format({ :controller => 'foo', @@ -69,21 +69,21 @@ module ActionDispatch def test_extras_are_not_included_if_optional path = Path::Pattern.from_string '/page/:id(/:action)' - route = Route.new("name", nil, path, { }, [], { :action => 'show' }) + route = Route.build("name", nil, path, { }, [], { :action => 'show' }) assert_equal '/page/10', route.format({ :id => 10 }) end def test_extras_are_not_included_if_optional_with_parameter path = Path::Pattern.from_string '(/sections/:section)/pages/:id' - route = Route.new("name", nil, path, { }, [], { :action => 'show' }) + route = Route.build("name", nil, path, { }, [], { :action => 'show' }) assert_equal '/pages/10', route.format({:id => 10}) end def test_extras_are_not_included_if_optional_parameter_is_nil path = Path::Pattern.from_string '(/sections/:section)/pages/:id' - route = Route.new("name", nil, path, { }, [], { :action => 'show' }) + route = Route.build("name", nil, path, { }, [], { :action => 'show' }) assert_equal '/pages/10', route.format({:id => 10, :section => nil}) end @@ -93,10 +93,10 @@ module ActionDispatch defaults = {:controller=>"pages", :action=>"show"} path = Path::Pattern.from_string "/page/:id(/:action)(.:format)" - specific = Route.new "name", nil, path, constraints, [:controller, :action], defaults + specific = Route.build "name", nil, path, constraints, [:controller, :action], defaults path = Path::Pattern.from_string "/:controller(/:action(/:id))(.:format)" - generic = Route.new "name", nil, path, constraints, [], {} + generic = Route.build "name", nil, path, constraints, [], {} knowledge = {:id=>20, :controller=>"pages", :action=>"show"} diff --git a/actionview/lib/action_view/helpers/asset_url_helper.rb b/actionview/lib/action_view/helpers/asset_url_helper.rb index 6ac8185b0f..717b326740 100644 --- a/actionview/lib/action_view/helpers/asset_url_helper.rb +++ b/actionview/lib/action_view/helpers/asset_url_helper.rb @@ -48,10 +48,10 @@ module ActionView # It is also possible the combination of additional connection overhead # (DNS, SSL) and the overall browser connection limits may result in this # solution being slower. You should be sure to measure your actual - # performance across targeted browers both before and after this change. + # performance across targeted browsers both before and after this change. # # To implement the corresponding hosts you can either setup four actual - # hosts or use wildcard DNS to CNAME the wilcard to a single asset host. + # hosts or use wildcard DNS to CNAME the wildcard to a single asset host. # You can read more about setting up your DNS CNAME records from your ISP. # # Note: This is purely a browser performance optimization and is not meant diff --git a/actionview/lib/action_view/helpers/sanitize_helper.rb b/actionview/lib/action_view/helpers/sanitize_helper.rb index a2e9f37453..191a881de0 100644 --- a/actionview/lib/action_view/helpers/sanitize_helper.rb +++ b/actionview/lib/action_view/helpers/sanitize_helper.rb @@ -120,7 +120,7 @@ module ActionView attr_writer :full_sanitizer, :link_sanitizer, :white_list_sanitizer # Vendors the full, link and white list sanitizers. - # Provided strictly for compabitility and can be removed in Rails 5. + # Provided strictly for compatibility and can be removed in Rails 5. def sanitizer_vendor Rails::Html::Sanitizer end diff --git a/actionview/test/abstract_unit.rb b/actionview/test/abstract_unit.rb index 35f520ed1c..12870acaa6 100644 --- a/actionview/test/abstract_unit.rb +++ b/actionview/test/abstract_unit.rb @@ -16,6 +16,7 @@ silence_warnings do end require 'active_support/testing/autorun' +require 'active_support/testing/method_call_assertions' require 'action_controller' require 'action_view' require 'action_view/testing/resolvers' @@ -281,3 +282,6 @@ def jruby_skip(message = '') end require 'mocha/setup' # FIXME: stop using mocha +class ActiveSupport::TestCase + include ActiveSupport::Testing::MethodCallAssertions +end diff --git a/actionview/test/template/asset_tag_helper_test.rb b/actionview/test/template/asset_tag_helper_test.rb index 01fc66bed6..496b33b35e 100644 --- a/actionview/test/template/asset_tag_helper_test.rb +++ b/actionview/test/template/asset_tag_helper_test.rb @@ -588,11 +588,13 @@ class AssetTagHelperTest < ActionView::TestCase end end - @controller.request.stubs(:ssl?).returns(false) - assert_equal "http://assets15.example.com/images/xml.png", image_path("xml.png") + @controller.request.stub(:ssl?, false) do + assert_equal "http://assets15.example.com/images/xml.png", image_path("xml.png") + end - @controller.request.stubs(:ssl?).returns(true) - assert_equal "http://localhost/images/xml.png", image_path("xml.png") + @controller.request.stub(:ssl?, true) do + assert_equal "http://localhost/images/xml.png", image_path("xml.png") + end end end diff --git a/actionview/test/template/form_helper_test.rb b/actionview/test/template/form_helper_test.rb index 37f2d7cad6..aef137935a 100644 --- a/actionview/test/template/form_helper_test.rb +++ b/actionview/test/template/form_helper_test.rb @@ -2003,21 +2003,22 @@ class FormHelperTest < ActionView::TestCase def test_form_for_with_remote_without_html @post.persisted = false - @post.stubs(:to_key).returns(nil) - form_for(@post, remote: true) do |f| - concat f.text_field(:title) - concat f.text_area(:body) - concat f.check_box(:secret) - end + @post.stub(:to_key, nil) do + form_for(@post, remote: true) do |f| + concat f.text_field(:title) + concat f.text_area(:body) + concat f.check_box(:secret) + end - expected = whole_form("/posts", "new_post", "new_post", remote: true) do - "<input name='post[title]' type='text' id='post_title' value='Hello World' />" + - "<textarea name='post[body]' id='post_body'>\nBack to the hill and over it again!</textarea>" + - "<input name='post[secret]' type='hidden' value='0' />" + - "<input name='post[secret]' checked='checked' type='checkbox' id='post_secret' value='1' />" - end + expected = whole_form("/posts", "new_post", "new_post", remote: true) do + "<input name='post[title]' type='text' id='post_title' value='Hello World' />" + + "<textarea name='post[body]' id='post_body'>\nBack to the hill and over it again!</textarea>" + + "<input name='post[secret]' type='hidden' value='0' />" + + "<input name='post[secret]' checked='checked' type='checkbox' id='post_secret' value='1' />" + end - assert_dom_equal expected, output_buffer + assert_dom_equal expected, output_buffer + end end def test_form_for_without_object @@ -2220,16 +2221,17 @@ class FormHelperTest < ActionView::TestCase def test_submit_with_object_as_new_record_and_locale_strings with_locale :submit do @post.persisted = false - @post.stubs(:to_key).returns(nil) - form_for(@post) do |f| - concat f.submit - end + @post.stub(:to_key, nil) do + form_for(@post) do |f| + concat f.submit + end - expected = whole_form('/posts', 'new_post', 'new_post') do - "<input name='commit' data-disable-with='Create Post' type='submit' value='Create Post' />" - end + expected = whole_form('/posts', 'new_post', 'new_post') do + "<input name='commit' data-disable-with='Create Post' type='submit' value='Create Post' />" + end - assert_dom_equal expected, output_buffer + assert_dom_equal expected, output_buffer + end end end @@ -2807,11 +2809,12 @@ class FormHelperTest < ActionView::TestCase def test_nested_fields_label_translation_with_more_than_10_records @post.comments = Array.new(11) { |id| Comment.new(id + 1) } - I18n.expects(:t).with('post.comments.body', default: [:"comment.body", ''], scope: "helpers.label").times(11).returns "Write body here" - - form_for(@post) do |f| - f.fields_for(:comments) do |cf| - concat cf.label(:body) + params = 11.times.map { ['post.comments.body', default: [:"comment.body", ''], scope: "helpers.label"] } + assert_called_with(I18n, :t, params, returns: "Write body here") do + form_for(@post) do |f| + f.fields_for(:comments) do |cf| + concat cf.label(:body) + end end end end diff --git a/actionview/test/template/form_options_helper_i18n_test.rb b/actionview/test/template/form_options_helper_i18n_test.rb index 4972ea6511..26ede09a5f 100644 --- a/actionview/test/template/form_options_helper_i18n_test.rb +++ b/actionview/test/template/form_options_helper_i18n_test.rb @@ -14,8 +14,9 @@ class FormOptionsHelperI18nTests < ActionView::TestCase end def test_select_with_prompt_true_translates_prompt_message - I18n.expects(:translate).with('helpers.select.prompt', { :default => 'Please select' }) - select('post', 'category', [], :prompt => true) + assert_called_with(I18n, :translate, ['helpers.select.prompt', { :default => 'Please select' }]) do + select('post', 'category', [], :prompt => true) + end end def test_select_with_translated_prompt diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index 4f7823045e..14dafd7d63 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -108,10 +108,11 @@ class LookupContextTest < ActiveSupport::TestCase end test "found templates respects given formats if one cannot be found from template or handler" do - ActionView::Template::Handlers::Builder.expects(:default_format).returns(nil) - @lookup_context.formats = [:text] - template = @lookup_context.find("hello", %w(test)) - assert_equal [:text], template.formats + assert_called(ActionView::Template::Handlers::Builder, :default_format, returns: nil) do + @lookup_context.formats = [:text] + template = @lookup_context.find("hello", %w(test)) + assert_equal [:text], template.formats + end end test "adds fallbacks to view paths when required" do @@ -210,45 +211,50 @@ end class LookupContextWithFalseCaching < ActiveSupport::TestCase def setup @resolver = ActionView::FixtureResolver.new("test/_foo.erb" => ["Foo", Time.utc(2000)]) - ActionView::Resolver.stubs(:caching?).returns(false) @lookup_context = ActionView::LookupContext.new(@resolver, {}) end test "templates are always found in the resolver but timestamp is checked before being compiled" do - 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 the timestamp is the same. - @resolver.hash["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 - template = @lookup_context.find("foo", %w(test), true) - assert_equal "Bar", template.source + ActionView::Resolver.stub(:caching?, false) do + 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 the timestamp is the same. + @resolver.hash["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 + template = @lookup_context.find("foo", %w(test), true) + assert_equal "Bar", template.source + end end test "if no template was found in the second lookup, with no cache, raise error" do - template = @lookup_context.find("foo", %w(test), true) - assert_equal "Foo", template.source + ActionView::Resolver.stub(:caching?, false) do + template = @lookup_context.find("foo", %w(test), true) + assert_equal "Foo", template.source - @resolver.hash.clear - assert_raise ActionView::MissingTemplate do - @lookup_context.find("foo", %w(test), true) + @resolver.hash.clear + assert_raise ActionView::MissingTemplate do + @lookup_context.find("foo", %w(test), true) + end end end test "if no template was cached in the first lookup, retrieval should work in the second call" do - @resolver.hash.clear - assert_raise ActionView::MissingTemplate do - @lookup_context.find("foo", %w(test), true) - end + ActionView::Resolver.stub(:caching?, false) do + @resolver.hash.clear + assert_raise ActionView::MissingTemplate do + @lookup_context.find("foo", %w(test), true) + end - @resolver.hash["test/_foo.erb"] = ["Foo", Time.utc(2000)] - template = @lookup_context.find("foo", %w(test), true) - assert_equal "Foo", template.source + @resolver.hash["test/_foo.erb"] = ["Foo", Time.utc(2000)] + template = @lookup_context.find("foo", %w(test), true) + assert_equal "Foo", template.source + end end end diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index d3b51cd629..921011b073 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -118,15 +118,17 @@ class TestERBTemplate < ActiveSupport::TestCase def test_refresh_with_templates @template = new_template("Hello", :virtual_path => "test/foo/bar") @template.locals = [:key] - @context.lookup_context.expects(:find_template).with("bar", %w(test/foo), false, [:key]).returns("template") - assert_equal "template", @template.refresh(@context) + assert_called_with(@context.lookup_context, :find_template,["bar", %w(test/foo), false, [:key]], returns: "template") do + assert_equal "template", @template.refresh(@context) + end end def test_refresh_with_partials @template = new_template("Hello", :virtual_path => "test/_foo") @template.locals = [:key] - @context.lookup_context.expects(:find_template).with("foo", %w(test), true, [:key]).returns("partial") - assert_equal "partial", @template.refresh(@context) + assert_called_with(@context.lookup_context, :find_template,[ "foo", %w(test), true, [:key]], returns: "partial") do + assert_equal "partial", @template.refresh(@context) + end end def test_refresh_raises_an_error_without_virtual_path diff --git a/actionview/test/template/test_case_test.rb b/actionview/test/template/test_case_test.rb index 80fbaa8392..b057d43ee0 100644 --- a/actionview/test/template/test_case_test.rb +++ b/actionview/test/template/test_case_test.rb @@ -20,6 +20,7 @@ module ActionView class TestCase helper ASharedTestHelper + DeveloperStruct = Struct.new(:name) module SharedTests def self.included(test_case) @@ -50,7 +51,7 @@ module ActionView end test "works without testing a helper module" do - assert_equal 'Eloy', render('developers/developer', :developer => stub(:name => 'Eloy')) + assert_equal 'Eloy', render('developers/developer', :developer => DeveloperStruct.new('Eloy')) end test "can render a layout with block" do @@ -69,13 +70,15 @@ module ActionView end test "delegates notice to request.flash[:notice]" do - view.request.flash.expects(:[]).with(:notice) - view.notice + assert_called_with(view.request.flash, :[], [:notice]) do + view.notice + end end test "delegates alert to request.flash[:alert]" do - view.request.flash.expects(:[]).with(:alert) - view.alert + assert_called_with(view.request.flash, :[], [:alert]) do + view.alert + end end test "uses controller lookup context" do @@ -119,7 +122,7 @@ module ActionView test "helper class that is being tested is always included in view instance" do @controller.controller_path = 'test' - @customers = [stub(:name => 'Eloy'), stub(:name => 'Manfred')] + @customers = [DeveloperStruct.new('Eloy'), DeveloperStruct.new('Manfred')] assert_match(/Hello: EloyHello: Manfred/, render(:partial => 'test/from_helper')) end end @@ -255,15 +258,15 @@ module ActionView end test "is able to render partials with local variables" do - assert_equal 'Eloy', render('developers/developer', :developer => stub(:name => 'Eloy')) + assert_equal 'Eloy', render('developers/developer', :developer => DeveloperStruct.new('Eloy')) assert_equal 'Eloy', render(:partial => 'developers/developer', - :locals => { :developer => stub(:name => 'Eloy') }) + :locals => { :developer => DeveloperStruct.new('Eloy') }) end test "is able to render partials from templates and also use instance variables" do @controller.controller_path = "test" - @customers = [stub(:name => 'Eloy'), stub(:name => 'Manfred')] + @customers = [DeveloperStruct.new('Eloy'), DeveloperStruct.new('Manfred')] assert_match(/Hello: EloyHello: Manfred/, render(:file => 'test/list')) end @@ -272,7 +275,7 @@ module ActionView view - @customers = [stub(:name => 'Eloy'), stub(:name => 'Manfred')] + @customers = [DeveloperStruct.new('Eloy'), DeveloperStruct.new('Manfred')] assert_match(/Hello: EloyHello: Manfred/, render(:file => 'test/list')) end diff --git a/actionview/test/template/translation_helper_test.rb b/actionview/test/template/translation_helper_test.rb index 5dc281adb2..749d0dd7fd 100644 --- a/actionview/test/template/translation_helper_test.rb +++ b/actionview/test/template/translation_helper_test.rb @@ -42,14 +42,16 @@ class TranslationHelperTest < ActiveSupport::TestCase end def test_delegates_setting_to_i18n - I18n.expects(:translate).with(:foo, :locale => 'en', :raise => true).returns("") - translate :foo, :locale => 'en' + assert_called_with(I18n, :translate, [:foo, :locale => 'en', :raise => true], returns: "") do + translate :foo, :locale => 'en' + end end def test_delegates_localize_to_i18n @time = Time.utc(2008, 7, 8, 12, 18, 38) - I18n.expects(:localize).with(@time) - localize @time + assert_called_with(I18n, :localize, [@time]) do + localize @time + end end def test_returns_missing_translation_message_wrapped_into_span @@ -125,8 +127,9 @@ class TranslationHelperTest < ActiveSupport::TestCase end def test_translate_escapes_interpolations_in_translations_with_a_html_suffix + word_struct = Struct.new(:to_s) assert_equal '<a>Hello <World></a>', translate(:'translations.interpolated_html', :word => '<World>') - assert_equal '<a>Hello <World></a>', translate(:'translations.interpolated_html', :word => stub(:to_s => "<World>")) + assert_equal '<a>Hello <World></a>', translate(:'translations.interpolated_html', :word => word_struct.new("<World>")) end def test_translate_with_html_count diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index bd86dfb88e..d880470d97 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,26 @@ +* Only try to nullify has_one target association if the record is persisted. + + Fixes #21223. + + *Agis Anastasopoulos* + +* Uniqueness validator raises descriptive error when running on a persisted + record without primary key. + + Closes #21304. + + *Yves Senn* + +* Add a native JSON data type support in MySQL. + + Example: + + create_table :json_data_type do |t| + t.json :settings + end + + *Ryuta Kamizono* + * Descriptive error message when fixtures contain a missing column. Closes #21201. diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index b5a8c81fe4..29c711082a 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -227,6 +227,31 @@ module ActiveRecord @association.last(*args) end + # Gives a record (or N records if a parameter is supplied) from the collection + # using the same rules as <tt>ActiveRecord::Base.take</tt>. + # + # class Person < ActiveRecord::Base + # has_many :pets + # end + # + # person.pets + # # => [ + # # #<Pet id: 1, name: "Fancy-Fancy", person_id: 1>, + # # #<Pet id: 2, name: "Spook", person_id: 1>, + # # #<Pet id: 3, name: "Choo-Choo", person_id: 1> + # # ] + # + # person.pets.take # => #<Pet id: 1, name: "Fancy-Fancy", person_id: 1> + # + # person.pets.take(2) + # # => [ + # # #<Pet id: 1, name: "Fancy-Fancy", person_id: 1>, + # # #<Pet id: 2, name: "Spook", person_id: 1> + # # ] + # + # another_person_without.pets # => [] + # another_person_without.pets.take # => nil + # another_person_without.pets.take(2) # => [] def take(n = nil) @association.take(n) end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 5a92bc5e8a..1829453d73 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -65,7 +65,7 @@ module ActiveRecord when :destroy target.destroy when :nullify - target.update_columns(reflection.foreign_key => nil) + target.update_columns(reflection.foreign_key => nil) if target.persisted? end end end diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 6ecc741195..3992a240b9 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -116,7 +116,7 @@ module ActiveRecord when String preloaders_for_one(association.to_sym, records, scope) else - raise ArgumentError, "#{association.inspect} was not recognised for preload" + raise ArgumentError, "#{association.inspect} was not recognized for preload" end end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index dbb0e2fab2..d0de42d27c 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -234,7 +234,7 @@ module ActiveRecord super end - # Marks this record to be destroyed as part of the parents save transaction. + # Marks this record to be destroyed as part of the parent's save transaction. # This does _not_ actually destroy the record instantly, rather child record will be destroyed # when <tt>parent.save</tt> is called. # @@ -243,7 +243,7 @@ module ActiveRecord @marked_for_destruction = true end - # Returns whether or not this record will be destroyed as part of the parents save transaction. + # Returns whether or not this record will be destroyed as part of the parent's save transaction. # # Only useful if the <tt>:autosave</tt> option on the parent is enabled for this associated model. def marked_for_destruction? 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 d17e272ed1..3115e03ea2 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -214,6 +214,7 @@ module ActiveRecord @name = name end + # Returns an array of ColumnDefinition objects for the columns of the table. def columns; @columns_hash.values; end # Returns a ColumnDefinition for the column with name +name+. @@ -369,6 +370,8 @@ module ActiveRecord self end + # remove the column +name+ from the table. + # remove_column(:account_id) def remove_column(name) @columns_hash.delete name.to_s end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 56227ddd80..ed14c781c6 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -266,6 +266,11 @@ module ActiveRecord false end + # Does this adapter support json data type? + def supports_json? + false + end + # This is meant to be implemented by the adapters that support extensions def disable_extension(name) end 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 af156c9c78..b3e55a0b90 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -10,6 +10,10 @@ module ActiveRecord options[:auto_increment] = true if type == :bigint super end + + def json(*args, **options) + args.each { |name| column(name, :json, options) } + end end class ColumnDefinition < ActiveRecord::ConnectionAdapters::ColumnDefinition @@ -242,17 +246,19 @@ module ActiveRecord QUOTED_TRUE, QUOTED_FALSE = '1', '0' NATIVE_DATABASE_TYPES = { - :primary_key => "int(11) auto_increment PRIMARY KEY", - :string => { :name => "varchar", :limit => 255 }, - :text => { :name => "text" }, - :integer => { :name => "int", :limit => 4 }, - :float => { :name => "float" }, - :decimal => { :name => "decimal" }, - :datetime => { :name => "datetime" }, - :time => { :name => "time" }, - :date => { :name => "date" }, - :binary => { :name => "blob" }, - :boolean => { :name => "tinyint", :limit => 1 } + primary_key: "int(11) auto_increment PRIMARY KEY", + string: { name: "varchar", limit: 255 }, + text: { name: "text" }, + integer: { name: "int", limit: 4 }, + float: { name: "float" }, + decimal: { name: "decimal" }, + datetime: { name: "datetime" }, + time: { name: "time" }, + date: { name: "date" }, + binary: { name: "blob" }, + boolean: { name: "tinyint", limit: 1 }, + bigint: { name: "bigint" }, + json: { name: "json" }, } INDEX_TYPES = [:fulltext, :spatial] @@ -790,6 +796,7 @@ module ActiveRecord m.register_type %r(longblob)i, Type::Binary.new(limit: 2**32 - 1) m.register_type %r(^float)i, Type::Float.new(limit: 24) m.register_type %r(^double)i, Type::Float.new(limit: 53) + m.register_type %r(^json)i, MysqlJson.new register_integer_type m, %r(^bigint)i, limit: 8 register_integer_type m, %r(^int)i, limit: 4 @@ -1043,6 +1050,14 @@ module ActiveRecord end end + class MysqlJson < Type::Internal::AbstractJson # :nodoc: + def changed_in_place?(raw_old_value, new_value) + # Normalization is required because MySQL JSON data format includes + # the space between the elements. + super(serialize(deserialize(raw_old_value)), new_value) + end + end + class MysqlString < Type::String # :nodoc: def serialize(value) case value @@ -1063,6 +1078,8 @@ module ActiveRecord end end + ActiveRecord::Type.register(:json, MysqlJson, adapter: :mysql) + ActiveRecord::Type.register(:json, MysqlJson, adapter: :mysql2) ActiveRecord::Type.register(:string, MysqlString, adapter: :mysql) ActiveRecord::Type.register(:string, MysqlString, adapter: :mysql2) end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index b7db57c9fe..ff43c7ec42 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -41,6 +41,10 @@ module ActiveRecord true end + def supports_json? + version >= '5.7.8' + end + # HELPER METHODS =========================================== def each_hash(result) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb index 8e1256baad..dbc879ffd4 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb @@ -2,32 +2,7 @@ module ActiveRecord module ConnectionAdapters module PostgreSQL module OID # :nodoc: - class Json < Type::Value # :nodoc: - include Type::Helpers::Mutable - - def type - :json - end - - def deserialize(value) - if value.is_a?(::String) - ::ActiveSupport::JSON.decode(value) rescue nil - else - value - end - end - - def serialize(value) - if value.is_a?(::Array) || value.is_a?(::Hash) - ::ActiveSupport::JSON.encode(value) - else - value - end - end - - def accessor - ActiveRecord::Store::StringKeyedHashAccessor - end + class Json < Type::Internal::AbstractJson end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 2c43c46a3d..27291bd2ea 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -201,6 +201,10 @@ module ActiveRecord true end + def supports_json? + postgresql_version >= 90200 + end + def index_algorithms { concurrently: 'CONCURRENTLY' } end diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index d589620f8a..718f04871d 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -218,11 +218,12 @@ module ActiveRecord class UnknownPrimaryKey < ActiveRecordError attr_reader :model - def initialize(model) - super("Unknown primary key for table #{model.table_name} in model #{model}.") + def initialize(model, description = nil) + message = "Unknown primary key for table #{model.table_name} in model #{model}." + message += "\n#{description}" if description + super(message) @model = model end - end # Raised when a relation cannot be mutated because it's already loaded. diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index b7b508d853..04c6124eef 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -376,6 +376,7 @@ module ActiveRecord attr_accessor :delegate # :nodoc: attr_accessor :disable_ddl_transaction # :nodoc: + # Raises <tt>ActiveRecord::PendingMigrationError</tt> error if any migrations are pending. def check_pending!(connection = Base.connection) raise ActiveRecord::PendingMigrationError if ActiveRecord::Migrator.needs_migration?(connection) end @@ -408,7 +409,10 @@ module ActiveRecord new.migrate direction end - # Disable DDL transactions for this migration. + # Disable the transaction wrapping this migration. + # You can still create your own transactions even after calling #disable_ddl_transaction! + # + # For more details read the {"Transactional Migrations" section above}[rdoc-ref:Migration]. def disable_ddl_transaction! @disable_ddl_transaction = true end @@ -810,7 +814,7 @@ module ActiveRecord new(:up, migrations, target_version).migrate end - def down(migrations_paths, target_version = nil, &block) + def down(migrations_paths, target_version = nil) migrations = migrations(migrations_paths) migrations.select! { |m| yield m } if block_given? diff --git a/activerecord/lib/active_record/type.rb b/activerecord/lib/active_record/type.rb index 2c0cda69d0..53f3b53bec 100644 --- a/activerecord/lib/active_record/type.rb +++ b/activerecord/lib/active_record/type.rb @@ -20,6 +20,8 @@ require 'active_record/type/adapter_specific_registry' require 'active_record/type/type_map' require 'active_record/type/hash_lookup_type_map' +require 'active_record/type/internal/abstract_json' + module ActiveRecord module Type @registry = AdapterSpecificRegistry.new diff --git a/activerecord/lib/active_record/type/internal/abstract_json.rb b/activerecord/lib/active_record/type/internal/abstract_json.rb new file mode 100644 index 0000000000..963a8245d0 --- /dev/null +++ b/activerecord/lib/active_record/type/internal/abstract_json.rb @@ -0,0 +1,33 @@ +module ActiveRecord + module Type + module Internal # :nodoc: + class AbstractJson < Type::Value # :nodoc: + include Type::Helpers::Mutable + + def type + :json + end + + def deserialize(value) + if value.is_a?(::String) + ::ActiveSupport::JSON.decode(value) rescue nil + else + value + end + end + + def serialize(value) + if value.is_a?(::Array) || value.is_a?(::Hash) + ::ActiveSupport::JSON.encode(value) + else + value + end + end + + def accessor + ActiveRecord::Store::StringKeyedHashAccessor + end + end + end + end +end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 32d17a1392..5706bbd903 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -18,7 +18,11 @@ module ActiveRecord relation = build_relation(finder_class, table, attribute, value) if record.persisted? && finder_class.primary_key.to_s != attribute.to_s - relation = relation.where.not(finder_class.primary_key => record.id) + if finder_class.primary_key + relation = relation.where.not(finder_class.primary_key => record.id) + else + raise UnknownPrimaryKey.new(finder_class, "Can not validate uniqueness for persisted record without primary key.") + end end relation = scope_relation(record, table, relation) relation = relation.merge(options[:conditions]) if options[:conditions] diff --git a/activerecord/test/cases/adapters/mysql2/json_test.rb b/activerecord/test/cases/adapters/mysql2/json_test.rb new file mode 100644 index 0000000000..c8c933af5e --- /dev/null +++ b/activerecord/test/cases/adapters/mysql2/json_test.rb @@ -0,0 +1,172 @@ +require 'cases/helper' +require 'support/schema_dumping_helper' + +if ActiveRecord::Base.connection.supports_json? +class Mysql2JSONTest < ActiveRecord::Mysql2TestCase + include SchemaDumpingHelper + self.use_transactional_tests = false + + class JsonDataType < ActiveRecord::Base + self.table_name = 'json_data_type' + + store_accessor :settings, :resolution + end + + def setup + @connection = ActiveRecord::Base.connection + begin + @connection.create_table('json_data_type') do |t| + t.json 'payload' + t.json 'settings' + end + end + end + + def teardown + @connection.drop_table :json_data_type, if_exists: true + JsonDataType.reset_column_information + end + + def test_column + column = JsonDataType.columns_hash["payload"] + assert_equal :json, column.type + assert_equal 'json', column.sql_type + + type = JsonDataType.type_for_attribute("payload") + assert_not type.binary? + end + + def test_change_table_supports_json + @connection.change_table('json_data_type') do |t| + t.json 'users' + end + JsonDataType.reset_column_information + column = JsonDataType.columns_hash['users'] + assert_equal :json, column.type + end + + def test_schema_dumping + output = dump_table_schema("json_data_type") + assert_match(/t\.json\s+"settings"/, output) + end + + def test_cast_value_on_write + x = JsonDataType.new payload: {"string" => "foo", :symbol => :bar} + assert_equal({"string" => "foo", :symbol => :bar}, x.payload_before_type_cast) + assert_equal({"string" => "foo", "symbol" => "bar"}, x.payload) + x.save + assert_equal({"string" => "foo", "symbol" => "bar"}, x.reload.payload) + end + + def test_type_cast_json + type = JsonDataType.type_for_attribute("payload") + + data = "{\"a_key\":\"a_value\"}" + hash = type.deserialize(data) + assert_equal({'a_key' => 'a_value'}, hash) + assert_equal({'a_key' => 'a_value'}, type.deserialize(data)) + + assert_equal({}, type.deserialize("{}")) + assert_equal({'key'=>nil}, type.deserialize('{"key": null}')) + assert_equal({'c'=>'}','"a"'=>'b "a b'}, type.deserialize(%q({"c":"}", "\"a\"":"b \"a b"}))) + end + + def test_rewrite + @connection.execute "insert into json_data_type (payload) VALUES ('{\"k\":\"v\"}')" + x = JsonDataType.first + x.payload = { '"a\'' => 'b' } + assert x.save! + end + + def test_select + @connection.execute "insert into json_data_type (payload) VALUES ('{\"k\":\"v\"}')" + x = JsonDataType.first + assert_equal({'k' => 'v'}, x.payload) + end + + def test_select_multikey + @connection.execute %q|insert into json_data_type (payload) VALUES ('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')| + x = JsonDataType.first + assert_equal({'k1' => 'v1', 'k2' => 'v2', 'k3' => [1,2,3]}, x.payload) + end + + def test_null_json + @connection.execute %q|insert into json_data_type (payload) VALUES(null)| + x = JsonDataType.first + assert_equal(nil, x.payload) + end + + def test_select_array_json_value + @connection.execute %q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')| + x = JsonDataType.first + assert_equal(['v0', {'k1' => 'v1'}], x.payload) + end + + def test_rewrite_array_json_value + @connection.execute %q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')| + x = JsonDataType.first + x.payload = ['v1', {'k2' => 'v2'}, 'v3'] + assert x.save! + end + + def test_with_store_accessors + x = JsonDataType.new(resolution: "320×480") + assert_equal "320×480", x.resolution + + x.save! + x = JsonDataType.first + assert_equal "320×480", x.resolution + + x.resolution = "640×1136" + x.save! + + x = JsonDataType.first + assert_equal "640×1136", x.resolution + end + + def test_duplication_with_store_accessors + x = JsonDataType.new(resolution: "320×480") + assert_equal "320×480", x.resolution + + y = x.dup + assert_equal "320×480", y.resolution + end + + def test_yaml_round_trip_with_store_accessors + x = JsonDataType.new(resolution: "320×480") + assert_equal "320×480", x.resolution + + y = YAML.load(YAML.dump(x)) + assert_equal "320×480", y.resolution + end + + def test_changes_in_place + json = JsonDataType.new + assert_not json.changed? + + json.payload = { 'one' => 'two' } + assert json.changed? + assert json.payload_changed? + + json.save! + assert_not json.changed? + + json.payload['three'] = 'four' + assert json.payload_changed? + + json.save! + json.reload + + assert_equal({ 'one' => 'two', 'three' => 'four' }, json.payload) + assert_not json.changed? + end + + def test_assigning_invalid_json + json = JsonDataType.new + + json.payload = 'foo' + + assert_nil json.payload + end +end +end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index ffbf60e390..8ed7b4e39f 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1325,6 +1325,14 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_match message, error.message end + test "preload with invalid argument" do + exception = assert_raises(ArgumentError) do + Author.preload(10).to_a + end + assert_equal('10 was not recognized for preload', exception.message) + end + + test "preloading readonly association" do # has-one firm = Firm.where(id: "1").preload(:readonly_account).first! diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 5a8afaf4d2..d46e7ad235 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -107,6 +107,14 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_nil Account.find(old_account_id).firm_id end + def test_nullification_on_destroyed_association + developer = Developer.create!(name: "Someone") + ship = Ship.create!(name: "Planet Caravan", developer: developer) + ship.destroy + assert !ship.persisted? + assert !developer.persisted? + end + def test_natural_assignment_to_nil_after_destroy firm = companies(:rails_core) old_account_id = firm.account.id diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index b2f209fe97..11e42f28dc 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -132,13 +132,9 @@ class MigrationTest < ActiveRecord::TestCase Person.connection.drop_table :testings2, if_exists: true end - def connection - ActiveRecord::Base.connection - end - def test_migration_instance_has_connection migration = Class.new(ActiveRecord::Migration).new - assert_equal connection, migration.connection + assert_equal ActiveRecord::Base.connection, migration.connection end def test_method_missing_delegates_to_connection diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 42e7507631..cdf63957f4 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -17,6 +17,7 @@ require 'models/minivan' require 'models/owner' require 'models/person' require 'models/pet' +require 'models/ship' require 'models/toy' require 'rexml/document' diff --git a/activerecord/test/cases/readonly_test.rb b/activerecord/test/cases/readonly_test.rb index 1c919f0b57..5f6eb41240 100644 --- a/activerecord/test/cases/readonly_test.rb +++ b/activerecord/test/cases/readonly_test.rb @@ -7,6 +7,7 @@ require 'models/computer' require 'models/project' require 'models/reader' require 'models/person' +require 'models/ship' class ReadOnlyTest < ActiveRecord::TestCase fixtures :authors, :posts, :comments, :developers, :projects, :developers_projects, :people, :readers diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index ceca2c8366..7502a55391 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -4,6 +4,7 @@ require 'models/reply' require 'models/warehouse_thing' require 'models/guid' require 'models/event' +require 'models/dashboard' class Wizard < ActiveRecord::Base self.abstract_class = true @@ -446,4 +447,26 @@ class UniquenessValidationTest < ActiveRecord::TestCase key2.key_number = 10 assert_not key2.valid? end + + def test_validate_uniqueness_without_primary_key + klass = Class.new(ActiveRecord::Base) do + self.table_name = "dashboards" + + validates_uniqueness_of :dashboard_id + + def self.name; "Dashboard" end + end + + abc = klass.create!(dashboard_id: "abc") + assert klass.new(dashboard_id: "xyz").valid? + assert_not klass.new(dashboard_id: "abc").valid? + + abc.dashboard_id = "def" + + e = assert_raises ActiveRecord::UnknownPrimaryKey do + abc.save! + end + assert_match(/\AUnknown primary key for table dashboards in model/, e.message) + assert_match(/Can not validate uniqueness for persisted record without primary key.\z/, e.message) + end end diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index d2a5a7fc49..8ac7a9df6a 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -50,6 +50,7 @@ class Developer < ActiveRecord::Base has_many :firms, :through => :contracts, :source => :firm has_many :comments, ->(developer) { where(body: "I'm #{developer.name}") } has_many :ratings, through: :comments + has_one :ship, dependent: :nullify scope :jamises, -> { where(:name => 'Jamis') } diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb index 95172e4d3e..e333b964ab 100644 --- a/activerecord/test/models/ship.rb +++ b/activerecord/test/models/ship.rb @@ -3,6 +3,7 @@ class Ship < ActiveRecord::Base belongs_to :pirate belongs_to :update_only_pirate, :class_name => 'Pirate' + belongs_to :developer, dependent: :destroy has_many :parts, :class_name => 'ShipPart' has_many :treasures diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 6f34115534..994ece9244 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -673,6 +673,7 @@ ActiveRecord::Schema.define do create_table :ships, force: true do |t| t.string :name t.integer :pirate_id + t.belongs_to :developer t.integer :update_only_pirate_id # Conventionally named column for counter_cache t.integer :treasures_count, default: 0 diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 837974bc85..8253a76383 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -26,7 +26,7 @@ module ActiveSupport end class << self - # Creates a new CacheStore object according to the given options. + # Creates a new Store object according to the given options. # # If no arguments are passed to this method, then a new # ActiveSupport::Cache::MemoryStore object will be returned. diff --git a/activesupport/lib/active_support/core_ext/file/atomic.rb b/activesupport/lib/active_support/core_ext/file/atomic.rb index fad6fa8d9d..ab635f6db8 100644 --- a/activesupport/lib/active_support/core_ext/file/atomic.rb +++ b/activesupport/lib/active_support/core_ext/file/atomic.rb @@ -8,43 +8,45 @@ class File # file.write('hello') # end # - # If your temp directory is not on the same filesystem as the file you're - # trying to write, you can provide a different temporary directory. + # This method needs to create a temporary file. By default it will create it + # in the same directory as the destination file. If you don't like this + # behaviour you can provide a different directory but it must be on the + # same physical filesystem as the the file you're trying to write. # # File.atomic_write('/data/something.important', '/data/tmp') do |file| # file.write('hello') # end - def self.atomic_write(file_name, temp_dir = Dir.tmpdir) + def self.atomic_write(file_name, temp_dir = dirname(file_name)) require 'tempfile' unless defined?(Tempfile) - require 'fileutils' unless defined?(FileUtils) - temp_file = Tempfile.new(basename(file_name), temp_dir) - temp_file.binmode - return_val = yield temp_file - temp_file.close + Tempfile.open(".#{basename(file_name)}", temp_dir) do |temp_file| + temp_file.binmode + return_val = yield temp_file + temp_file.close - if File.exist?(file_name) - # Get original file permissions - old_stat = stat(file_name) - else - # If not possible, probe which are the default permissions in the - # destination directory. - old_stat = probe_stat_in(dirname(file_name)) - end - - # Overwrite original file with temp file - FileUtils.mv(temp_file.path, file_name) + old_stat = if exist?(file_name) + # Get original file permissions + stat(file_name) + elsif temp_dir != dirname(file_name) + # If not possible, probe which are the default permissions in the + # destination directory. + probe_stat_in(dirname(file_name)) + end - # Set correct permissions on new file - begin - chown(old_stat.uid, old_stat.gid, file_name) - # This operation will affect filesystem ACL's - chmod(old_stat.mode, file_name) + if old_stat + # Set correct permissions on new file + begin + chown(old_stat.uid, old_stat.gid, temp_file.path) + # This operation will affect filesystem ACL's + chmod(old_stat.mode, temp_file.path) + rescue Errno::EPERM, Errno::EACCES + # Changing file ownership failed, moving on. + end + end - # Make sure we return the result of the yielded block + # Overwrite original file with temp file + rename(temp_file.path, file_name) return_val - rescue Errno::EPERM, Errno::EACCES - # Changing file ownership failed, moving on. end end diff --git a/activesupport/lib/active_support/core_ext/object/try.rb b/activesupport/lib/active_support/core_ext/object/try.rb index 69be6c4abc..c67eb25b68 100644 --- a/activesupport/lib/active_support/core_ext/object/try.rb +++ b/activesupport/lib/active_support/core_ext/object/try.rb @@ -8,7 +8,7 @@ module ActiveSupport def try!(*a, &b) if a.empty? && block_given? - if b.arity.zero? + if b.arity == 0 instance_eval(&b) else yield self diff --git a/activesupport/lib/active_support/log_subscriber/test_helper.rb b/activesupport/lib/active_support/log_subscriber/test_helper.rb index 70ac4a4d5c..cbc20c103d 100644 --- a/activesupport/lib/active_support/log_subscriber/test_helper.rb +++ b/activesupport/lib/active_support/log_subscriber/test_helper.rb @@ -11,6 +11,7 @@ module ActiveSupport # include ActiveSupport::LogSubscriber::TestHelper # # def setup + # super # ActiveRecord::LogSubscriber.attach_to(:active_record) # end # diff --git a/activesupport/lib/active_support/testing/method_call_assertions.rb b/activesupport/lib/active_support/testing/method_call_assertions.rb index 517f02e3ef..fccaa54f40 100644 --- a/activesupport/lib/active_support/testing/method_call_assertions.rb +++ b/activesupport/lib/active_support/testing/method_call_assertions.rb @@ -1,11 +1,13 @@ +require 'minitest/mock' + module ActiveSupport module Testing module MethodCallAssertions # :nodoc: private - def assert_called(object, method_name, message = nil, times: 1) + def assert_called(object, method_name, message = nil, times: 1, returns: nil) times_called = 0 - object.stub(method_name, -> { times_called += 1 }) { yield } + object.stub(method_name, proc { times_called += 1; returns }) { yield } error = "Expected #{method_name} to be called #{times} times, " \ "but was called #{times_called} times" diff --git a/activesupport/test/testing/method_call_assertions_test.rb b/activesupport/test/testing/method_call_assertions_test.rb index 652cbda8da..3e5ba7c079 100644 --- a/activesupport/test/testing/method_call_assertions_test.rb +++ b/activesupport/test/testing/method_call_assertions_test.rb @@ -27,6 +27,18 @@ class MethodCallAssertionsTest < ActiveSupport::TestCase end end + def test_assert_called_method_with_arguments + assert_called(@object, :<<) do + @object << 2 + end + end + + def test_assert_called_returns + assert_called(@object, :increment, returns: 10) do + assert_equal 10, @object.increment + end + end + def test_assert_called_failure error = assert_raises(Minitest::Assertion) do assert_called(@object, :increment) do diff --git a/guides/CHANGELOG.md b/guides/CHANGELOG.md index fd177b4238..09fb7b1a0e 100644 --- a/guides/CHANGELOG.md +++ b/guides/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add code of conduct to contributing guide + + *Jon Moss* + * New section in Configuring: Configuring Active Job *Eliot Sykes* diff --git a/guides/bug_report_templates/action_controller_master.rb b/guides/bug_report_templates/action_controller_master.rb index 1a4b736348..31a3f6c650 100644 --- a/guides/bug_report_templates/action_controller_master.rb +++ b/guides/bug_report_templates/action_controller_master.rb @@ -9,6 +9,7 @@ gemfile(true) do source 'https://rubygems.org' gem 'rails', github: 'rails/rails' gem 'arel', github: 'rails/arel' + gem 'rack', github: 'rack/rack' end require 'action_controller/railtie' diff --git a/guides/bug_report_templates/active_record_master.rb b/guides/bug_report_templates/active_record_master.rb index 270dbe7df7..6ae50b2460 100644 --- a/guides/bug_report_templates/active_record_master.rb +++ b/guides/bug_report_templates/active_record_master.rb @@ -9,6 +9,7 @@ gemfile(true) do source 'https://rubygems.org' gem 'rails', github: 'rails/rails' gem 'arel', github: 'rails/arel' + gem 'rack', github: 'rack/rack' gem 'sqlite3' end diff --git a/guides/bug_report_templates/generic_master.rb b/guides/bug_report_templates/generic_master.rb index b6b4562751..f6dce64280 100644 --- a/guides/bug_report_templates/generic_master.rb +++ b/guides/bug_report_templates/generic_master.rb @@ -9,6 +9,7 @@ gemfile(true) do source 'https://rubygems.org' gem 'rails', github: 'rails/rails' gem 'arel', github: 'rails/arel' + gem 'rack', github: 'rack/rack' end require 'active_support' diff --git a/guides/source/4_2_release_notes.md b/guides/source/4_2_release_notes.md index 684bd286bc..8a59007420 100644 --- a/guides/source/4_2_release_notes.md +++ b/guides/source/4_2_release_notes.md @@ -227,6 +227,17 @@ restore the old behavior. If you do this, be sure to configure your firewall properly such that only trusted machines on your network can access your development server. +### Changed status option symbols for `render` + +Due to a [change in Rack](https://github.com/rack/rack/commit/be28c6a2ac152fe4adfbef71f3db9f4200df89e8), the symbols that the `render` method accepts for the `:status` option have changed: + +- 306: `:reserved` has been removed. +- 413: `:request_entity_too_large` has been renamed to `:payload_too_large`. +- 414: `:request_uri_too_long` has been renamed to `:uri_too_long`. +- 416: `:requested_range_not_satisfiable` has been renamed to `:range_not_satisfiable`. + +Keep in mind that if calling `render` with an unknown symbol, the response status will default to 500. + ### HTML Sanitizer The HTML sanitizer has been replaced with a new, more robust, implementation diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index 09fbdc0d32..d2173c39f6 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -1174,7 +1174,7 @@ end WARNING: You shouldn't do `rescue_from Exception` or `rescue_from StandardError` unless you have a particular reason as it will cause serious side-effects (e.g. you won't be able to see exception details and tracebacks during development). -NOTE: Certain exceptions are only rescuable from the `ApplicationController` class, as they are raised before the controller gets initialized and the action gets executed. See Pratik Naik's [article](http://m.onkey.org/2008/7/20/rescue-from-dispatching) on the subject for more information. +NOTE: Certain exceptions are only rescuable from the `ApplicationController` class, as they are raised before the controller gets initialized and the action gets executed. Force HTTPS protocol -------------------- diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md index db7eeed19a..00c41a480e 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -1443,12 +1443,12 @@ Sanitizes a block of CSS code. Strips all link tags from text leaving just the link text. ```ruby -strip_links("<a href="http://rubyonrails.org">Ruby on Rails</a>") +strip_links('<a href="http://rubyonrails.org">Ruby on Rails</a>') # => Ruby on Rails ``` ```ruby -strip_links("emails to <a href="mailto:me@email.com">me@email.com</a>.") +strip_links('emails to <a href="mailto:me@email.com">me@email.com</a>.') # => emails to me@email.com. ``` diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 4b4d70d3ce..c0a4a0ba39 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -341,8 +341,6 @@ User.find_each(begin_at: 2000, batch_size: 5000) do |user| end ``` -Another example would be if you wanted multiple workers handling the same processing queue. You could have each worker handle 10000 records by setting the appropriate `:begin_at` option on each worker. - **`:end_at`** Similar to the `:begin_at` option, `:end_at` allows you to configure the last ID of the sequence whenever the highest ID is not the one you need. @@ -356,6 +354,10 @@ User.find_each(begin_at: 2000, end_at: 10000, batch_size: 5000) do |user| end ``` +Another example would be if you wanted multiple workers handling the same +processing queue. You could have each worker handle 10000 records by setting the +appropriate `:begin_at` and `:end_at` options on each worker. + #### `find_in_batches` The `find_in_batches` method is similar to `find_each`, since both retrieve batches of records. The difference is that `find_in_batches` yields _batches_ to the block as an array of models, instead of individually. The following example will yield to the supplied block an array of up to 1000 invoices at a time, with the final block containing any remaining invoices: diff --git a/guides/source/active_support_instrumentation.md b/guides/source/active_support_instrumentation.md index cd44c685ba..e5a560edd0 100644 --- a/guides/source/active_support_instrumentation.md +++ b/guides/source/active_support_instrumentation.md @@ -395,6 +395,38 @@ INFO. Cache stores may add their own keys } ``` +Active Job +-------- + +### enqueue_at.active_job + +| Key | Value | +| ------------ | -------------------------------------- | +| `:adapter` | QueueAdapter object processing the job | +| `:job` | Job object | + +### enqueue.active_job + +| Key | Value | +| ------------ | -------------------------------------- | +| `:adapter` | QueueAdapter object processing the job | +| `:job` | Job object | + +### perform_start.active_job + +| Key | Value | +| ------------ | -------------------------------------- | +| `:adapter` | QueueAdapter object processing the job | +| `:job` | Job object | + +### perform.active_job + +| Key | Value | +| ------------ | -------------------------------------- | +| `:adapter` | QueueAdapter object processing the job | +| `:job` | Job object | + + Railties -------- diff --git a/guides/source/api_app.md b/guides/source/api_app.md index 29ca872254..1652d91086 100644 --- a/guides/source/api_app.md +++ b/guides/source/api_app.md @@ -367,7 +367,7 @@ controller modules by default: methods returning `ActionDispatch::Request` and `ActionDispatch::Response` objects. - `ActionController::DataStreaming`: Support for `send_file` and `send_data`. -- `AbstractController::Callbacks`: Support for `before_filter` and friends. +- `AbstractController::Callbacks`: Support for `before_action` and friends. - `ActionController::Instrumentation`: Support for the instrumentation hooks defined by Action Controller (see [the instrumentation guide](active_support_instrumentation.html#action-controller)). diff --git a/guides/source/api_documentation_guidelines.md b/guides/source/api_documentation_guidelines.md index 46c9013087..a4feff798d 100644 --- a/guides/source/api_documentation_guidelines.md +++ b/guides/source/api_documentation_guidelines.md @@ -84,6 +84,11 @@ English Please use American English (*color*, *center*, *modularize*, etc). See [a list of American and British English spelling differences here](http://en.wikipedia.org/wiki/American_and_British_English_spelling_differences). +Comma +------- + +Please use the Oxford comma (*red, white, and blue* style). See [the detail of Oxford comma](http://en.wikipedia.org/wiki/Serial_comma). + Example Code ------------ diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index 3b944f1274..ba82713266 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -15,6 +15,9 @@ After reading this guide, you will know: Ruby on Rails is not "someone else's framework." Over the years, hundreds of people have contributed to Ruby on Rails ranging from a single character to massive architectural changes or significant documentation - all with the goal of making Ruby on Rails better for everyone. Even if you don't feel up to writing code or documentation yet, there are a variety of other ways that you can contribute, from reporting issues to testing patches. +As mentioned in [Rails +README](https://github.com/rails/rails/blob/master/README.md), everyone interacting in Rails and its sub-project’s codebases, issue trackers, chat rooms, and mailing lists is expected to follow the Rails [code of conduct](https://github.com/rails/rails/blob/master/CODE_OF_CONDUCT.md). + -------------------------------------------------------------------------------- Reporting an Issue diff --git a/guides/source/debugging_rails_applications.md b/guides/source/debugging_rails_applications.md index 44434c164b..fe8757fbd5 100644 --- a/guides/source/debugging_rails_applications.md +++ b/guides/source/debugging_rails_applications.md @@ -346,23 +346,13 @@ by asking the debugger for help. Type: `help` ``` (byebug) help -byebug 2.7.0 + h[elp][ <cmd>[ <subcmd>]] -Type 'help <command-name>' for help on a specific command - -Available commands: -backtrace delete enable help list pry next restart source up -break disable eval info method ps save step var -catch display exit interrupt next putl set thread -condition down finish irb p quit show trace -continue edit frame kill pp reload skip undisplay + help -- prints this help. + help <cmd> -- prints help on command <cmd>. + help <cmd> <subcmd> -- prints help on <cmd>'s subcommand <subcmd>. ``` -TIP: To view the help menu for any command use `help <command-name>` at the -debugger prompt. For example: _`help list`_. You can abbreviate any debugging -command by supplying just enough letters to distinguish them from other -commands. For example, you can use `l` for the `list` command. - To see the previous ten lines you should type `list-` (or `l-`). ``` @@ -469,12 +459,12 @@ The debugger can list, stop, resume and switch between running threads by using the `thread` command (or the abbreviated `th`). This command has a handful of options: -* `thread` shows the current thread. -* `thread list` is used to list all threads and their statuses. The plus + +* `thread`: shows the current thread. +* `thread list`: is used to list all threads and their statuses. The plus + character and the number indicates the current thread of execution. -* `thread stop _n_` stop thread _n_. -* `thread resume _n_` resumes thread _n_. -* `thread switch _n_` switches the current thread context to _n_. +* `thread stop _n_`: stop thread _n_. +* `thread resume _n_`: resumes thread _n_. +* `thread switch _n_`: switches the current thread context to _n_. This command is very helpful when you are debugging concurrent threads and need to verify that there are no race conditions in your code. @@ -630,13 +620,15 @@ Processing by ArticlesController#index as HTML (byebug) ``` -If we use `next`, we want go deep inside method calls. Instead, byebug will go -to the next line within the same context. In this case, this is the last line of -the method, so `byebug` will jump to next next line of the previous frame. - +If we use `next`, we won't go deep inside method calls. Instead, `byebug` will +go to the next line within the same context. In this case, it is the last line +of the current method, so `byebug` will return to the next line of the caller ++method. ``` (byebug) next -Next went up a frame because previous frame finished + +Next advances to the next line (line 6: `end`), which returns to the next line +of the caller method: [4, 13] in /PathTo/project/test_app/app/controllers/articles_controller.rb 4: # GET /articles @@ -653,8 +645,8 @@ Next went up a frame because previous frame finished (byebug) ``` -If we use `step` in the same situation, we will literally go to the next Ruby -instruction to be executed. In this case, Active Support's `week` method. +If we use `step` in the same situation, `byebug` will literally go to the next +Ruby instruction to be executed -- in this case, Active Support's `week` method. ``` (byebug) step @@ -752,12 +744,12 @@ To list all active catchpoints use `catch`. There are two ways to resume execution of an application that is stopped in the debugger: -* `continue` [line-specification] \(or `c`): resume program execution, at the +* `continue [line-specification]` \(or `c`): resume program execution, at the address where your script last stopped; any breakpoints set at that address are bypassed. The optional argument line-specification allows you to specify a line number to set a one-time breakpoint which is deleted when that breakpoint is reached. -* `finish` [frame-number] \(or `fin`): execute until the selected stack frame +* `finish [frame-number]` \(or `fin`): execute until the selected stack frame returns. If no frame number is given, the application will run until the currently selected frame returns. The currently selected frame starts out the most-recent frame or 0 if no frame positioning (e.g up, down or frame) has been @@ -773,8 +765,8 @@ environment variable. A specific _line_ can also be given. ### Quitting -To exit the debugger, use the `quit` command (abbreviated `q`), or its alias -`exit`. +To exit the debugger, use the `quit` command (abbreviated to `q`). Or, type `q!` +to bypass the `Really quit? (y/n)` prompt and exit unconditionally. A simple quit tries to terminate all threads in effect. Therefore your server will be stopped and you will have to start it again. diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md index 94cd7297e2..b425eb126a 100644 --- a/guides/source/layouts_and_rendering.md +++ b/guides/source/layouts_and_rendering.md @@ -360,7 +360,6 @@ Rails understands both numeric status codes and the corresponding symbols shown | | 303 | :see_other | | | 304 | :not_modified | | | 305 | :use_proxy | -| | 306 | :reserved | | | 307 | :temporary_redirect | | | 308 | :permanent_redirect | | **Client Error** | 400 | :bad_request | @@ -376,10 +375,10 @@ Rails understands both numeric status codes and the corresponding symbols shown | | 410 | :gone | | | 411 | :length_required | | | 412 | :precondition_failed | -| | 413 | :request_entity_too_large | -| | 414 | :request_uri_too_long | +| | 413 | :payload_too_large | +| | 414 | :uri_too_long | | | 415 | :unsupported_media_type | -| | 416 | :requested_range_not_satisfiable | +| | 416 | :range_not_satisfiable | | | 417 | :expectation_failed | | | 422 | :unprocessable_entity | | | 423 | :locked | diff --git a/guides/source/security.md b/guides/source/security.md index c701027479..93c270064a 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -198,11 +198,10 @@ This attack method works by including malicious code or a link in a page that ac In the [session chapter](#sessions) you have learned that most Rails applications use cookie-based sessions. Either they store the session id in the cookie and have a server-side session hash, or the entire session hash is on the client-side. In either case the browser will automatically send along the cookie on every request to a domain, if it can find a cookie for that domain. The controversial point is, that it will also send the cookie, if the request comes from a site of a different domain. Let's start with an example: -* Bob browses a message board and views a post from a hacker where there is a crafted HTML image element. The element references a command in Bob's project management application, rather than an image file. -* `<img src="http://www.webapp.com/project/1/destroy">` -* Bob's session at www.webapp.com is still alive, because he didn't log out a few minutes ago. -* By viewing the post, the browser finds an image tag. It tries to load the suspected image from www.webapp.com. As explained before, it will also send along the cookie with the valid session id. -* The web application at www.webapp.com verifies the user information in the corresponding session hash and destroys the project with the ID 1. It then returns a result page which is an unexpected result for the browser, so it will not display the image. +* Bob browses a message board and views a post from a hacker where there is a crafted HTML image element. The element references a command in Bob's project management application, rather than an image file: `<img src="http://www.webapp.com/project/1/destroy">` +* Bob's session at `www.webapp.com` is still alive, because he didn't log out a few minutes ago. +* By viewing the post, the browser finds an image tag. It tries to load the suspected image from `www.webapp.com`. As explained before, it will also send along the cookie with the valid session id. +* The web application at `www.webapp.com` verifies the user information in the corresponding session hash and destroys the project with the ID 1. It then returns a result page which is an unexpected result for the browser, so it will not display the image. * Bob doesn't notice the attack - but a few days later he finds out that project number one is gone. It is important to notice that the actual crafted image or link doesn't necessarily have to be situated in the web application's domain, it can be anywhere - in a forum, blog post or email. @@ -227,7 +226,7 @@ The HTTP protocol basically provides two main types of requests - GET and POST ( If your web application is RESTful, you might be used to additional HTTP verbs, such as PATCH, PUT or DELETE. Most of today's web browsers, however do not support them - only GET and POST. Rails uses a hidden `_method` field to handle this barrier. -_POST requests can be sent automatically, too_. Here is an example for a link which displays www.harmless.com as destination in the browser's status bar. In fact it dynamically creates a new form that sends a POST request. +_POST requests can be sent automatically, too_. Here is an example for a link which displays `www.harmless.com` as destination in the browser's status bar. In fact it dynamically creates a new form that sends a POST request. ```html <a href="http://www.harmless.com/" onclick=" @@ -761,7 +760,7 @@ s = sanitize(user_input, tags: tags, attributes: %w(href title)) This allows only the given tags and does a good job, even against all kinds of tricks and malformed tags. -As a second step, _it is good practice to escape all output of the application_, especially when re-displaying user input, which hasn't been input-filtered (as in the search form example earlier on). _Use `escapeHTML()` (or its alias `h()`) method_ to replace the HTML input characters &, ", <, > by their uninterpreted representations in HTML (`&`, `"`, `<`;, and `>`). However, it can easily happen that the programmer forgets to use it, so _it is recommended to use the SafeErb gem. SafeErb reminds you to escape strings from external sources. +As a second step, _it is good practice to escape all output of the application_, especially when re-displaying user input, which hasn't been input-filtered (as in the search form example earlier on). _Use `escapeHTML()` (or its alias `h()`) method_ to replace the HTML input characters &, ", <, and > by their uninterpreted representations in HTML (`&`, `"`, `<`, and `>`). However, it can easily happen that the programmer forgets to use it, so _it is recommended to use the SafeErb gem. SafeErb reminds you to escape strings from external sources. ##### Obfuscation and Encoding Injection @@ -1015,18 +1014,12 @@ config.action_dispatch.default_headers.clear Here is a list of common headers: -* X-Frame-Options -_'SAMEORIGIN' in Rails by default_ - allow framing on same domain. Set it to 'DENY' to deny framing at all or 'ALLOWALL' if you want to allow framing for all website. -* X-XSS-Protection -_'1; mode=block' in Rails by default_ - use XSS Auditor and block page if XSS attack is detected. Set it to '0;' if you want to switch XSS Auditor off(useful if response contents scripts from request parameters) -* X-Content-Type-Options -_'nosniff' in Rails by default_ - stops the browser from guessing the MIME type of a file. -* X-Content-Security-Policy -[A powerful mechanism for controlling which sites certain content types can be loaded from](http://w3c.github.io/webappsec/specs/content-security-policy/csp-specification.dev.html) -* Access-Control-Allow-Origin -Used to control which sites are allowed to bypass same origin policies and send cross-origin requests. -* Strict-Transport-Security -[Used to control if the browser is allowed to only access a site over a secure connection](http://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) +* **X-Frame-Options:** _'SAMEORIGIN' in Rails by default_ - allow framing on same domain. Set it to 'DENY' to deny framing at all or 'ALLOWALL' if you want to allow framing for all website. +* **X-XSS-Protection:** _'1; mode=block' in Rails by default_ - use XSS Auditor and block page if XSS attack is detected. Set it to '0;' if you want to switch XSS Auditor off(useful if response contents scripts from request parameters) +* **X-Content-Type-Options:** _'nosniff' in Rails by default_ - stops the browser from guessing the MIME type of a file. +* **X-Content-Security-Policy:** [A powerful mechanism for controlling which sites certain content types can be loaded from](http://w3c.github.io/webappsec/specs/content-security-policy/csp-specification.dev.html) +* **Access-Control-Allow-Origin:** Used to control which sites are allowed to bypass same origin policies and send cross-origin requests. +* **Strict-Transport-Security:** [Used to control if the browser is allowed to only access a site over a secure connection](http://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) Environmental Security ---------------------- diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 249fe96772..38074e80e0 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -209,13 +209,19 @@ module Rails [ GemfileEntry.path('rails', Rails::Generators::RAILS_DEV_PATH), GemfileEntry.github('sprockets-rails', 'rails/sprockets-rails'), - GemfileEntry.github('arel', 'rails/arel') + GemfileEntry.github('sprockets', 'rails/sprockets'), + GemfileEntry.github('sass-rails', 'rails/sass-rails'), + GemfileEntry.github('arel', 'rails/arel'), + GemfileEntry.github('rack', 'rack/rack') ] elsif options.edge? [ GemfileEntry.github('rails', 'rails/rails'), GemfileEntry.github('sprockets-rails', 'rails/sprockets-rails'), - GemfileEntry.github('arel', 'rails/arel') + GemfileEntry.github('sprockets', 'rails/sprockets'), + GemfileEntry.github('sass-rails', 'rails/sass-rails'), + GemfileEntry.github('arel', 'rails/arel'), + GemfileEntry.github('rack', 'rack/rack') ] else [GemfileEntry.version('rails', @@ -255,8 +261,6 @@ module Rails return [] if options[:skip_sprockets] gems = [] - gems << GemfileEntry.version('sass-rails', '~> 5.0', - 'Use SCSS for stylesheets') gems << GemfileEntry.version('uglifier', '>= 1.3.0', diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/manifest.js.tt b/railties/lib/rails/generators/rails/app/templates/app/assets/manifest.js.tt new file mode 100644 index 0000000000..3325553f57 --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/app/assets/manifest.js.tt @@ -0,0 +1,8 @@ + +<% unless options.api? -%> +//= link_tree ./images +<% end -%> +<% unless options.skip_javascript -%> +//= link ./javascripts/application.js +<% end -%> +//= link ./stylesheets/application.css diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index 0e1326ce4f..8fea30189e 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -104,8 +104,9 @@ task default: :test end def test_dummy_assets - template "rails/javascripts.js", "#{dummy_path}/app/assets/javascripts/application.js", force: true - template "rails/stylesheets.css", "#{dummy_path}/app/assets/stylesheets/application.css", force: true + template "rails/javascripts.js", "#{dummy_path}/app/assets/javascripts/application.js", force: true + template "rails/stylesheets.css", "#{dummy_path}/app/assets/stylesheets/application.css", force: true + template "rails/dummy_manifest.js", "#{dummy_path}/app/assets/manifest.js", force: true end def test_dummy_clean @@ -122,6 +123,10 @@ task default: :test end end + def assets_manifest + template "rails/engine_manifest.js", "app/assets/#{underscored_name}_manifest.js" + end + def stylesheets if mountable? copy_file "rails/stylesheets.css", @@ -220,6 +225,10 @@ task default: :test build(:lib) end + def create_assets_manifest_file + build(:assets_manifest) unless api? + end + def create_public_stylesheets_files build(:stylesheets) unless api? end diff --git a/railties/lib/rails/generators/rails/plugin/templates/rails/dummy_manifest.js b/railties/lib/rails/generators/rails/plugin/templates/rails/dummy_manifest.js new file mode 100644 index 0000000000..ace695a811 --- /dev/null +++ b/railties/lib/rails/generators/rails/plugin/templates/rails/dummy_manifest.js @@ -0,0 +1,11 @@ + +<% unless api? -%> +//= link_tree ./images +<% end -%> +<% unless options.skip_javascript -%> +//= link ./javascripts/application.js +<% end -%> +//= link ./stylesheets/application.css +<% if mountable? && !api? -%> +//= link <%= underscored_name %>_manifest.js +<% end -%> diff --git a/railties/lib/rails/generators/rails/plugin/templates/rails/engine_manifest.js b/railties/lib/rails/generators/rails/plugin/templates/rails/engine_manifest.js new file mode 100644 index 0000000000..f8ef26982a --- /dev/null +++ b/railties/lib/rails/generators/rails/plugin/templates/rails/engine_manifest.js @@ -0,0 +1,6 @@ +<% if mountable? -%> +<% unless options.skip_javascript -%> +//= link ./javascripts/<%= namespaced_name %>/application.js +<% end -%> +//= link ./stylesheets/<%= namespaced_name %>/application.css +<% end -%> diff --git a/railties/lib/rails/source_annotation_extractor.rb b/railties/lib/rails/source_annotation_extractor.rb index 9b058a1848..8dd87b6cc5 100644 --- a/railties/lib/rails/source_annotation_extractor.rb +++ b/railties/lib/rails/source_annotation_extractor.rb @@ -3,7 +3,7 @@ # rake notes # rake notes:optimize # -# and friends. See <tt>rake -T notes</tt> and <tt>railties/lib/tasks/annotations.rake</tt>. +# and friends. See <tt>rake -T notes</tt> and <tt>railties/lib/rails/tasks/annotations.rake</tt>. # # Annotation objects are triplets <tt>:line</tt>, <tt>:tag</tt>, <tt>:text</tt> that # represent the line where the annotation lives, its tag, and its text. Note diff --git a/railties/test/application/asset_debugging_test.rb b/railties/test/application/asset_debugging_test.rb index 36ab8109a7..fef65adda2 100644 --- a/railties/test/application/asset_debugging_test.rb +++ b/railties/test/application/asset_debugging_test.rb @@ -48,7 +48,7 @@ module ApplicationTests assert_no_match(/<script src="\/assets\/xmlhr-([0-z]+)\.js"><\/script>/, last_response.body) end - test "assets aren't concatenated when compile is true is on and debug_assets params is true" do + test "assets are served with sourcemaps when compile is true and debug_assets params is true" do add_to_env_config "production", "config.assets.compile = true" ENV["RAILS_ENV"] = "production" @@ -57,8 +57,7 @@ module ApplicationTests class ::PostsController < ActionController::Base ; end get '/posts?debug_assets=true' - assert_match(/<script src="\/assets\/application(\.self)?-([0-z]+)\.js\?body=1"><\/script>/, last_response.body) - assert_match(/<script src="\/assets\/xmlhr(\.self)?-([0-z]+)\.js\?body=1"><\/script>/, last_response.body) + assert_match(/<script src="\/assets\/application(\.debug)?-([0-z]+)\.js"><\/script>/, last_response.body) end end end diff --git a/railties/test/application/assets_test.rb b/railties/test/application/assets_test.rb index ba2c27d8e8..ddcd51f3fc 100644 --- a/railties/test/application/assets_test.rb +++ b/railties/test/application/assets_test.rb @@ -72,6 +72,7 @@ module ApplicationTests end test "precompile creates the file, gives it the original asset's content and run in production as default" do + app_file "app/assets/manifest.js", "//= link_tree ./javascripts" app_file "app/assets/javascripts/application.js", "alert();" app_file "app/assets/javascripts/foo/application.js", "alert();" @@ -87,6 +88,7 @@ module ApplicationTests end def test_precompile_does_not_hit_the_database + app_file "app/assets/manifest.js", "//= link_tree ./javascripts" app_file "app/assets/javascripts/application.js", "alert();" app_file "app/assets/javascripts/foo/application.js", "alert();" app_file "app/controllers/users_controller.rb", <<-eoruby @@ -178,8 +180,11 @@ module ApplicationTests end test 'precompile use assets defined in app config and reassigned in app env config' do - add_to_config 'config.assets.precompile = [ "something.js" ]' - add_to_env_config 'production', 'config.assets.precompile += [ "another.js" ]' + add_to_config 'config.assets.precompile = [ "something_manifest.js" ]' + add_to_env_config 'production', 'config.assets.precompile += [ "another_manifest.js" ]' + + app_file 'app/assets/something_manifest.js', '//= link ./javascripts/something.js' + app_file 'app/assets/another_manifest.js', '//= link ./javascripts/another.js' app_file 'app/assets/javascripts/something.js.erb', 'alert();' app_file 'app/assets/javascripts/another.js.erb', 'alert();' @@ -190,14 +195,14 @@ module ApplicationTests assert_file_exists("#{app_path}/public/assets/another-*.js") end - test "asset pipeline should use a Sprockets::Index when config.assets.digest is true" do + test "asset pipeline should use a Sprockets::CachedEnvironment when config.assets.digest is true" do add_to_config "config.action_controller.perform_caching = false" add_to_env_config "production", "config.assets.compile = true" ENV["RAILS_ENV"] = "production" require "#{app_path}/config/environment" - assert_equal Sprockets::Index, Rails.application.assets.class + assert_equal Sprockets::CachedEnvironment, Rails.application.assets.class end test "precompile creates a manifest file with all the assets listed" do @@ -246,7 +251,7 @@ module ApplicationTests test "precompile properly refers files referenced with asset_path and runs in the provided RAILS_ENV" do app_file "app/assets/images/rails.png", "notactuallyapng" - app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>" + app_file "app/assets/stylesheets/application.css.erb", "p { background-image: url(<%= asset_path('rails.png') %>) }" add_to_env_config "test", "config.assets.digest = true" precompile!('RAILS_ENV=test') @@ -258,7 +263,7 @@ module ApplicationTests test "precompile shouldn't use the digests present in manifest.json" do app_file "app/assets/images/rails.png", "notactuallyapng" - app_file "app/assets/stylesheets/application.css.erb", "p { url: <%= asset_path('rails.png') %> }" + app_file "app/assets/stylesheets/application.css.erb", "p { background-image: url(<%= asset_path('rails.png') %>) }" ENV["RAILS_ENV"] = "production" precompile! @@ -277,7 +282,7 @@ module ApplicationTests test "precompile appends the md5 hash to files referenced with asset_path and run in production with digest true" do app_file "app/assets/images/rails.png", "notactuallyapng" - app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>" + app_file "app/assets/stylesheets/application.css.erb", "p { background-image: url(<%= asset_path('rails.png') %>) }" ENV["RAILS_ENV"] = "production" precompile! @@ -289,7 +294,8 @@ module ApplicationTests test "precompile should handle utf8 filenames" do filename = "レイルズ.png" app_file "app/assets/images/#{filename}", "not an image really" - add_to_config "config.assets.precompile = [ /\.png$/, /application.(css|js)$/ ]" + app_file "app/assets/manifest.js", "//= link_tree ./images" + add_to_config "config.assets.precompile = %w(manifest.js)" precompile! @@ -394,7 +400,7 @@ module ApplicationTests app_file "app/assets/javascripts/xmlhr.js.erb", "<%= Post.name %>" precompile! - assert_equal "Post;\n", File.read(Dir["#{app_path}/public/assets/application-*.js"].first) + assert_equal "Post\n;\n", File.read(Dir["#{app_path}/public/assets/application-*.js"].first) end test "initialization on the assets group should set assets_dir" do @@ -439,9 +445,9 @@ module ApplicationTests class ::PostsController < ActionController::Base; end get '/posts', {}, {'HTTPS'=>'off'} - assert_match('src="http://example.com/assets/application.self.js', last_response.body) + assert_match('src="http://example.com/assets/application.debug.js', last_response.body) get '/posts', {}, {'HTTPS'=>'on'} - assert_match('src="https://example.com/assets/application.self.js', last_response.body) + assert_match('src="https://example.com/assets/application.debug.js', last_response.body) end test "asset urls should be protocol-relative if no request is in scope" do diff --git a/railties/test/generators/actions_test.rb b/railties/test/generators/actions_test.rb index 2857dae07e..fabba555ef 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -1,7 +1,6 @@ require 'generators/generators_test_helper' require 'rails/generators/rails/app/app_generator' require 'env_helpers' -require 'minitest/mock' class ActionsTest < Rails::Generators::TestCase include GeneratorsTestHelper @@ -12,13 +11,11 @@ class ActionsTest < Rails::Generators::TestCase def setup Rails.application = TestApp::Application - @mock_generator = Minitest::Mock.new super end def teardown Rails.application = TestApp::Application.instance - @mock_generator.verify end def test_invoke_other_generator_with_shortcut @@ -150,16 +147,13 @@ class ActionsTest < Rails::Generators::TestCase end def test_git_with_symbol_should_run_command_using_git_scm - @mock_generator.expect(:call, nil, ['git init']) - generator.stub(:run, @mock_generator) do + assert_called_with(generator, :run, ['git init']) do action :git, :init end end def test_git_with_hash_should_run_each_command_using_git_scm - @mock_generator.expect(:call, nil, ["git rm README"]) - @mock_generator.expect(:call, nil, ["git add ."]) - generator.stub(:run, @mock_generator) do + assert_called_with(generator, :run, [ ["git rm README"], ["git add ."] ]) do action :git, rm: 'README', add: '.' end end @@ -185,15 +179,13 @@ class ActionsTest < Rails::Generators::TestCase end def test_generate_should_run_script_generate_with_argument_and_options - @mock_generator.expect(:call, nil, ['bin/rails generate model MyModel', verbose: false]) - generator.stub(:run_ruby_script, @mock_generator) do + assert_called_with(generator, :run_ruby_script, ['bin/rails generate model MyModel', verbose: false]) do action :generate, 'model', 'MyModel' end end def test_rake_should_run_rake_command_with_default_env - @mock_generator.expect(:call, nil, ["rake log:clear RAILS_ENV=development", verbose: false]) - generator.stub(:run, @mock_generator) do + assert_called_with(generator, :run, ["rake log:clear RAILS_ENV=development", verbose: false]) do with_rails_env nil do action :rake, 'log:clear' end @@ -201,15 +193,13 @@ class ActionsTest < Rails::Generators::TestCase end def test_rake_with_env_option_should_run_rake_command_in_env - @mock_generator.expect(:call, nil, ['rake log:clear RAILS_ENV=production', verbose: false]) - generator.stub(:run, @mock_generator) do + assert_called_with(generator, :run, ['rake log:clear RAILS_ENV=production', verbose: false]) do action :rake, 'log:clear', env: 'production' end end def test_rake_with_rails_env_variable_should_run_rake_command_in_env - @mock_generator.expect(:call, nil, ['rake log:clear RAILS_ENV=production', verbose: false]) - generator.stub(:run, @mock_generator) do + assert_called_with(generator, :run, ['rake log:clear RAILS_ENV=production', verbose: false]) do with_rails_env "production" do action :rake, 'log:clear' end @@ -217,8 +207,7 @@ class ActionsTest < Rails::Generators::TestCase end def test_env_option_should_win_over_rails_env_variable_when_running_rake - @mock_generator.expect(:call, nil, ['rake log:clear RAILS_ENV=production', verbose: false]) - generator.stub(:run, @mock_generator) do + assert_called_with(generator, :run, ['rake log:clear RAILS_ENV=production', verbose: false]) do with_rails_env "staging" do action :rake, 'log:clear', env: 'production' end @@ -226,8 +215,7 @@ class ActionsTest < Rails::Generators::TestCase end def test_rake_with_sudo_option_should_run_rake_command_with_sudo - @mock_generator.expect(:call, nil, ["sudo rake log:clear RAILS_ENV=development", verbose: false]) - generator.stub(:run, @mock_generator) do + assert_called_with(generator, :run, ["sudo rake log:clear RAILS_ENV=development", verbose: false]) do with_rails_env nil do action :rake, 'log:clear', sudo: true end @@ -235,8 +223,7 @@ class ActionsTest < Rails::Generators::TestCase end def test_capify_should_run_the_capify_command - @mock_generator.expect(:call, nil, ['capify .', verbose: false]) - generator.stub(:run, @mock_generator) do + assert_called_with(generator, :run, ['capify .', verbose: false]) do action :capify! end end @@ -274,8 +261,7 @@ F def test_readme run_generator - 2.times { @mock_generator.expect(:call, destination_root,[]) } - Rails::Generators::AppGenerator.stub(:source_root, @mock_generator) do + assert_called(Rails::Generators::AppGenerator, :source_root, times: 2, returns: destination_root) do assert_match "application up and running", action(:readme, "README.md") end end @@ -283,8 +269,7 @@ F def test_readme_with_quiet generator(default_arguments, quiet: true) run_generator - 2.times { @mock_generator.expect(:call, destination_root,[]) } - Rails::Generators::AppGenerator.stub(:source_root, @mock_generator) do + assert_called(Rails::Generators::AppGenerator, :source_root, times: 2, returns: destination_root) do assert_no_match "application up and running", action(:readme, "README.md") end end diff --git a/railties/test/generators/generators_test_helper.rb b/railties/test/generators/generators_test_helper.rb index 62ca0ecb4b..b19a5a7144 100644 --- a/railties/test/generators/generators_test_helper.rb +++ b/railties/test/generators/generators_test_helper.rb @@ -1,6 +1,7 @@ require 'abstract_unit' require 'active_support/core_ext/module/remove_method' require 'active_support/testing/stream' +require 'active_support/testing/method_call_assertions' require 'rails/generators' require 'rails/generators/test_case' @@ -25,6 +26,7 @@ require 'action_view' module GeneratorsTestHelper include ActiveSupport::Testing::Stream + include ActiveSupport::Testing::MethodCallAssertions def self.included(base) base.class_eval do diff --git a/railties/test/generators_test.rb b/railties/test/generators_test.rb index 31a575749a..291415858c 100644 --- a/railties/test/generators_test.rb +++ b/railties/test/generators_test.rb @@ -1,7 +1,6 @@ require 'generators/generators_test_helper' require 'rails/generators/rails/model/model_generator' require 'rails/generators/test_unit/model/model_generator' -require 'minitest/mock' class GeneratorsTest < Rails::Generators::TestCase include GeneratorsTestHelper @@ -9,18 +8,15 @@ class GeneratorsTest < Rails::Generators::TestCase def setup @path = File.expand_path("lib", Rails.root) $LOAD_PATH.unshift(@path) - @mock_generator = MiniTest::Mock.new end def teardown $LOAD_PATH.delete(@path) - @mock_generator.verify end def test_simple_invoke assert File.exist?(File.join(@path, 'generators', 'model_generator.rb')) - @mock_generator.expect(:call, nil, [["Account"],{}]) - TestUnit::Generators::ModelGenerator.stub(:start, @mock_generator) do + assert_called_with(TestUnit::Generators::ModelGenerator, :start, [["Account"], {}]) do Rails::Generators.invoke("test_unit:model", ["Account"]) end end @@ -51,23 +47,20 @@ class GeneratorsTest < Rails::Generators::TestCase def test_should_give_higher_preference_to_rails_generators assert File.exist?(File.join(@path, 'generators', 'model_generator.rb')) - @mock_generator.expect(:call, nil, [["Account"],{}]) - Rails::Generators::ModelGenerator.stub(:start, @mock_generator) do + assert_called_with(Rails::Generators::ModelGenerator, :start, [["Account"], {}]) do warnings = capture(:stderr){ Rails::Generators.invoke :model, ["Account"] } assert warnings.empty? end end def test_invoke_with_default_values - @mock_generator.expect(:call, nil, [["Account"],{}]) - Rails::Generators::ModelGenerator.stub(:start, @mock_generator) do + assert_called_with(Rails::Generators::ModelGenerator, :start, [["Account"], {}]) do Rails::Generators.invoke :model, ["Account"] end end def test_invoke_with_config_values - @mock_generator.expect(:call, nil, [["Account"],{behavior: :skip}]) - Rails::Generators::ModelGenerator.stub(:start, @mock_generator) do + assert_called_with(Rails::Generators::ModelGenerator, :start, [["Account"], behavior: :skip]) do Rails::Generators.invoke :model, ["Account"], behavior: :skip end end @@ -115,8 +108,7 @@ class GeneratorsTest < Rails::Generators::TestCase def test_invoke_with_nested_namespaces model_generator = Minitest::Mock.new model_generator.expect(:start, nil, [["Account"], {}]) - @mock_generator.expect(:call, model_generator, ['namespace', 'my:awesome']) - Rails::Generators.stub(:find_by_namespace, @mock_generator) do + assert_called_with(Rails::Generators, :find_by_namespace, ['namespace', 'my:awesome'], returns: model_generator) do Rails::Generators.invoke 'my:awesome:namespace', ["Account"] end model_generator.verify @@ -185,8 +177,7 @@ class GeneratorsTest < Rails::Generators::TestCase def test_fallbacks_for_generators_on_invoke Rails::Generators.fallbacks[:shoulda] = :test_unit - @mock_generator.expect(:call, nil, [["Account"],{}]) - TestUnit::Generators::ModelGenerator.stub(:start, @mock_generator) do + assert_called_with(TestUnit::Generators::ModelGenerator, :start, [["Account"], {}]) do Rails::Generators.invoke "shoulda:model", ["Account"] end ensure @@ -196,8 +187,7 @@ class GeneratorsTest < Rails::Generators::TestCase def test_nested_fallbacks_for_generators Rails::Generators.fallbacks[:shoulda] = :test_unit Rails::Generators.fallbacks[:super_shoulda] = :shoulda - @mock_generator.expect(:call, nil, [["Account"],{}]) - TestUnit::Generators::ModelGenerator.stub(:start, @mock_generator) do + assert_called_with(TestUnit::Generators::ModelGenerator, :start, [["Account"], {}]) do Rails::Generators.invoke "super_shoulda:model", ["Account"] end ensure |