diff options
72 files changed, 837 insertions, 548 deletions
diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 88b0962e8c..2f9c0dd7a3 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1 +1,24 @@ +* Template lookup now respect default locale and I18n fallbacks. + + Given the following templates: + + mailer/demo.html.erb + mailer/demo.en.html.erb + mailer/demo.pt.html.erb + + Before this change for a locale that doesn't have its related file the `mailer/demo.html.erb` will + be rendered even if `en` is the default locale. + + Now `mailer/demo.en.html.erb` has precedence over the file without locale. + + Also, it is possible to give a fallback. + + mailer/demo.pt.html.erb + mailer/demo.pt-BR.html.erb + + So if the locale is `pt-PT`, `mailer/demo.pt.html.erb` will be rendered given the right I18n + fallback configuration. + + *Rafael Mendonça França* + Please check [4-2-stable](https://github.com/rails/rails/blob/4-2-stable/actionmailer/CHANGELOG.md) for previous changes. diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 021a758940..53cc1fdb31 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -586,8 +586,6 @@ module ActionMailer } ActiveSupport::Notifications.instrument("process.action_mailer", payload) do - lookup_context.skip_default_locale! - super @_message = NullMail.new unless @_mail_was_called end diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index 396d0a95b5..5d9eda2555 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -353,10 +353,35 @@ class BaseTest < ActiveSupport::TestCase assert_equal("text/plain", email.parts[0].mime_type) assert_equal("Implicit with locale PL TEXT", email.parts[0].body.encoded) assert_equal("text/html", email.parts[1].mime_type) - assert_equal("Implicit with locale HTML", email.parts[1].body.encoded) + assert_equal("Implicit with locale EN HTML", email.parts[1].body.encoded) end end + test "implicit multipart with fallback locale" do + fallback_backend = Class.new(I18n::Backend::Simple) do + include I18n::Backend::Fallbacks + end + + begin + backend = I18n.backend + I18n.backend = fallback_backend.new + I18n.fallbacks[:"de-AT"] = [:de] + + swap I18n, locale: 'de-AT' do + email = BaseMailer.implicit_with_locale + assert_equal(2, email.parts.size) + assert_equal("multipart/alternative", email.mime_type) + assert_equal("text/plain", email.parts[0].mime_type) + assert_equal("Implicit with locale DE-AT TEXT", email.parts[0].body.encoded) + assert_equal("text/html", email.parts[1].mime_type) + assert_equal("Implicit with locale DE HTML", email.parts[1].body.encoded) + end + ensure + I18n.backend = backend + end + end + + test "implicit multipart with several view paths uses the first one with template" do old = BaseMailer.view_paths begin diff --git a/actionmailer/test/fixtures/base_mailer/implicit_with_locale.de-AT.text.erb b/actionmailer/test/fixtures/base_mailer/implicit_with_locale.de-AT.text.erb new file mode 100644 index 0000000000..e97505fad9 --- /dev/null +++ b/actionmailer/test/fixtures/base_mailer/implicit_with_locale.de-AT.text.erb @@ -0,0 +1 @@ +Implicit with locale DE-AT TEXT
\ No newline at end of file diff --git a/actionmailer/test/fixtures/base_mailer/implicit_with_locale.de.html.erb b/actionmailer/test/fixtures/base_mailer/implicit_with_locale.de.html.erb new file mode 100644 index 0000000000..0536b5d3e2 --- /dev/null +++ b/actionmailer/test/fixtures/base_mailer/implicit_with_locale.de.html.erb @@ -0,0 +1 @@ +Implicit with locale DE HTML
\ No newline at end of file diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 98b573e21e..b5380a46e8 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,19 @@ +* Using `head` method returns empty response_body instead + of returning a single space " ". + + The old behavior was added as a workaround for a bug in an early + version of Safari, where the HTTP headers are not returned correctly + if the response body has a 0-length. This is been fixed since and + the workaround is no longer necessary. + + Fixes #18253. + + *Prathamesh Sonpatki* + +* Fix how polymorphic routes works with objects that implement `to_model`. + + *Travis Grathwell* + * Stop converting empty arrays in `params` to `nil` This behaviour was introduced in response to CVE-2012-2660, CVE-2012-2694 diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index 3d2badf9c2..57e60222cd 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -29,14 +29,14 @@ module ActionController self.status = status self.location = url_for(location) if location + self.response_body = "" + if include_content?(self._status_code) self.content_type = content_type || (Mime[formats.first] if formats) self.response.charset = false if self.response - self.response_body = " " else headers.delete('Content-Type') headers.delete('Content-Length') - self.response_body = "" end end diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index d9bd277d62..2e116ea9cd 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -247,7 +247,7 @@ module ActionDispatch args = [] model = record.to_model - name = if record.persisted? + name = if model.persisted? args << model model.model_name.singular_route_key else @@ -290,11 +290,12 @@ module ActionDispatch when Class @key_strategy.call record.model_name else - if record.persisted? - args << record.to_model - record.to_model.model_name.singular_route_key + model = record.to_model + if model.persisted? + args << model + model.model_name.singular_route_key else - @key_strategy.call record.to_model.model_name + @key_strategy.call model.model_name end end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index a9a1576fed..f0e2c5becc 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -480,6 +480,84 @@ module ActionDispatch # end # end # end + # + # Another longer example would be: + # + # A simple integration test that exercises multiple controllers: + # + # require 'test_helper' + # + # class UserFlowsTest < ActionDispatch::IntegrationTest + # test "login and browse site" do + # # login via https + # https! + # get "/login" + # assert_response :success + # + # post_via_redirect "/login", username: users(:david).username, password: users(:david).password + # assert_equal '/welcome', path + # assert_equal 'Welcome david!', flash[:notice] + # + # https!(false) + # get "/articles/all" + # assert_response :success + # assert assigns(:articles) + # end + # end + # + # As you can see the integration test involves multiple controllers and + # exercises the entire stack from database to dispatcher. In addition you can + # have multiple session instances open simultaneously in a test and extend + # those instances with assertion methods to create a very powerful testing + # DSL (domain-specific language) just for your application. + # + # Here's an example of multiple sessions and custom DSL in an integration test + # + # require 'test_helper' + # + # class UserFlowsTest < ActionDispatch::IntegrationTest + # test "login and browse site" do + # # User david logs in + # david = login(:david) + # # User guest logs in + # guest = login(:guest) + # + # # Both are now available in different sessions + # assert_equal 'Welcome david!', david.flash[:notice] + # assert_equal 'Welcome guest!', guest.flash[:notice] + # + # # User david can browse site + # david.browses_site + # # User guest can browse site as well + # guest.browses_site + # + # # Continue with other assertions + # end + # + # private + # + # module CustomDsl + # def browses_site + # get "/products/all" + # assert_response :success + # assert assigns(:products) + # end + # end + # + # def login(user) + # open_session do |sess| + # sess.extend(CustomDsl) + # u = users(user) + # sess.https! + # sess.post "/login", username: u.username, password: u.password + # assert_equal '/welcome', sess.path + # sess.https!(false) + # end + # end + # end + # + # Consult the Rails Testing Guide for more. + class IntegrationTest < ActiveSupport::TestCase include Integration::Runner include ActionController::TemplateAssertions diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 829729eb1b..2e08a6af9f 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -1003,21 +1003,21 @@ class YieldingAroundFiltersTest < ActionController::TestCase def test_first_action_in_multiple_before_action_chain_halts controller = ::FilterTest::TestMultipleFiltersController.new response = test_process(controller, 'fail_1') - assert_equal ' ', response.body + assert_equal '', response.body assert_equal 1, controller.instance_variable_get(:@try) end def test_second_action_in_multiple_before_action_chain_halts controller = ::FilterTest::TestMultipleFiltersController.new response = test_process(controller, 'fail_2') - assert_equal ' ', response.body + assert_equal '', response.body assert_equal 2, controller.instance_variable_get(:@try) end def test_last_action_in_multiple_before_action_chain_halts controller = ::FilterTest::TestMultipleFiltersController.new response = test_process(controller, 'fail_3') - assert_equal ' ', response.body + assert_equal '', response.body assert_equal 3, controller.instance_variable_get(:@try) end diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 729717608f..fb39ac7508 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1 +1,6 @@ +* Added an explicit error message, in `ActionView::PartialRenderer` + for partial `rendering`, when the value of option `as` has invalid characters. + + *Angelo Capilleri* + Please check [4-2-stable](https://github.com/rails/rails/blob/4-2-stable/actionview/CHANGELOG.md) for previous changes. diff --git a/actionview/lib/action_view/helpers/text_helper.rb b/actionview/lib/action_view/helpers/text_helper.rb index a9f1631586..2c40ed1832 100644 --- a/actionview/lib/action_view/helpers/text_helper.rb +++ b/actionview/lib/action_view/helpers/text_helper.rb @@ -309,7 +309,7 @@ module ActionView # <table> # <% @items.each do |item| %> # <tr class="<%= cycle("odd", "even") -%>"> - # <td>item</td> + # <td><%= item %></td> # </tr> # <% end %> # </table> diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index ea687d9cca..36855ec3d0 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -191,7 +191,6 @@ module ActionView def initialize(view_paths, details = {}, prefixes = []) @details, @details_key = {}, nil - @skip_default_locale = false @cache = true @prefixes = prefixes @rendered_format = nil @@ -213,12 +212,6 @@ module ActionView super(values) end - # Do not use the default locale on template lookup. - def skip_default_locale! - @skip_default_locale = true - self.locale = nil - end - # Override locale to return a symbol instead of array. def locale @details[:locale].first @@ -233,7 +226,7 @@ module ActionView config.locale = value end - super(@skip_default_locale ? I18n.locale : default_locale) + super(default_locale) end # Uses the first format in the formats array for layout lookup. diff --git a/actionview/lib/action_view/record_identifier.rb b/actionview/lib/action_view/record_identifier.rb index 63f645431a..c8484bed34 100644 --- a/actionview/lib/action_view/record_identifier.rb +++ b/actionview/lib/action_view/record_identifier.rb @@ -2,29 +2,54 @@ require 'active_support/core_ext/module' require 'action_view/model_naming' module ActionView - # The record identifier encapsulates a number of naming conventions for dealing with records, like Active Records or - # pretty much any other model type that has an id. These patterns are then used to try elevate the view actions to - # a higher logical level. + # RecordIdentifier encapsulates methods used by various ActionView helpers + # to associate records with DOM elements. # - # # routes - # resources :posts + # Consider for example the following code that displays the body of a post: # - # # view - # <%= div_for(post) do %> <div id="post_45" class="post"> - # <%= post.body %> What a wonderful world! - # <% end %> </div> + # <%= div_for(post) do %> + # <%= post.body %> + # <% end %> # - # # controller - # def update - # post = Post.find(params[:id]) - # post.update(params[:post]) + # When +post+ is a new, unsaved ActiveRecord::Base intance, the resulting HTML + # is: # - # redirect_to(post) # Calls polymorphic_url(post) which in turn calls post_url(post) - # end + # <div id="new_post" class="post"> + # </div> + # + # When +post+ is a persisted ActiveRecord::Base instance, the resulting HTML + # is: + # + # <div id="post_42" class="post"> + # What a wonderful world! + # </div> + # + # In both cases, the +id+ and +class+ of the wrapping DOM element are + # automatically generated, following naming conventions encapsulated by the + # RecordIdentifier methods #dom_id and #dom_class: + # + # dom_id(Post.new) # => "new_post" + # dom_class(Post.new) # => "post" + # dom_id(Post.find 42) # => "post_42" + # dom_class(Post.find 42) # => "post" # - # As the example above shows, you can stop caring to a large extent what the actual id of the post is. - # You just know that one is being assigned and that the subsequent calls in redirect_to expect that - # same naming convention and allows you to write less code if you follow it. + # Note that these methods do not strictly require +Post+ to be a subclass of + # ActiveRecord::Base. + # Any +Post+ class will work as long as its instances respond to +to_key+ + # and +model_name+, given that +model_name+ responds to +param_key+. + # For instance: + # + # class Post + # attr_accessor :to_key + # + # def model_name + # OpenStruct.new param_key: 'post' + # end + # + # def self.find(id) + # new.tap { |post| post.to_key = [id] } + # end + # end module RecordIdentifier extend self extend ModelNaming diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index f627d5d40c..610396506f 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -384,7 +384,7 @@ module ActionView end if as = options[:as] - raise_invalid_identifier(as) unless as.to_s =~ /\A[a-z_]\w*\z/ + raise_invalid_option_as(as) unless as.to_s =~ /\A[a-z_]\w*\z/ as = as.to_sym end @@ -530,11 +530,19 @@ module ActionView end IDENTIFIER_ERROR_MESSAGE = "The partial name (%s) is not a valid Ruby identifier; " + - "make sure your partial name starts with a lowercase letter or underscore, " + + "make sure your partial name starts with underscore, " + + "and is followed by any combination of letters, numbers and underscores." + + OPTION_AS_ERROR_MESSAGE = "The value (%s) of the option `as` is not a valid Ruby identifier; " + + "make sure it starts with lowercase letter, " + "and is followed by any combination of letters, numbers and underscores." def raise_invalid_identifier(path) raise ArgumentError.new(IDENTIFIER_ERROR_MESSAGE % (path)) end + + def raise_invalid_option_as(as) + raise ArgumentError.new(OPTION_AS_ERROR_MESSAGE % (as)) + end end end diff --git a/actionview/test/activerecord/polymorphic_routes_test.rb b/actionview/test/activerecord/polymorphic_routes_test.rb index 5842b775bb..8e1ed2776d 100644 --- a/actionview/test/activerecord/polymorphic_routes_test.rb +++ b/actionview/test/activerecord/polymorphic_routes_test.rb @@ -25,15 +25,17 @@ class Series < ActiveRecord::Base self.table_name = 'projects' end -class ModelDelegator < ActiveRecord::Base - self.table_name = 'projects' - +class ModelDelegator def to_model ModelDelegate.new end end class ModelDelegate + def persisted? + true + end + def model_name ActiveModel::Name.new(self.class) end @@ -605,13 +607,18 @@ class PolymorphicRoutesTest < ActionController::TestCase end end - def test_routing_a_to_model_delegate + def test_routing_to_a_model_delegate with_test_routes do - @delegator.save assert_url "http://example.com/model_delegates/overridden", @delegator end end + def test_nested_routing_to_a_model_delegate + with_test_routes do + assert_url "http://example.com/foo/model_delegates/overridden", [:foo, @delegator] + end + end + def with_namespaced_routes(name) with_routing do |set| set.draw do @@ -645,6 +652,9 @@ class PolymorphicRoutesTest < ActionController::TestCase end resources :series resources :model_delegates + namespace :foo do + resources :model_delegates + end end extend @routes.url_helpers diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index 4e502bede9..e580a477a6 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -175,14 +175,14 @@ module RenderTestCases def test_render_partial_with_invalid_name e = assert_raises(ArgumentError) { @view.render(:partial => "test/200") } assert_equal "The partial name (test/200) is not a valid Ruby identifier; " + - "make sure your partial name starts with a lowercase letter or underscore, " + + "make sure your partial name starts with underscore, " + "and is followed by any combination of letters, numbers and underscores.", e.message end def test_render_partial_with_missing_filename e = assert_raises(ArgumentError) { @view.render(:partial => "test/") } assert_equal "The partial name (test/) is not a valid Ruby identifier; " + - "make sure your partial name starts with a lowercase letter or underscore, " + + "make sure your partial name starts with underscore, " + "and is followed by any combination of letters, numbers and underscores.", e.message end @@ -194,7 +194,21 @@ module RenderTestCases def test_render_partial_with_hyphen e = assert_raises(ArgumentError) { @view.render(:partial => "test/a-in") } assert_equal "The partial name (test/a-in) is not a valid Ruby identifier; " + - "make sure your partial name starts with a lowercase letter or underscore, " + + "make sure your partial name starts with underscore, " + + "and is followed by any combination of letters, numbers and underscores.", e.message + end + + def test_render_partial_with_invalid_option_as + e = assert_raises(ArgumentError) { @view.render(:partial => "test/partial_only", :as => 'a-in') } + assert_equal "The value (a-in) of the option `as` is not a valid Ruby identifier; " + + "make sure it starts with lowercase letter, " + + "and is followed by any combination of letters, numbers and underscores.", e.message + end + + def test_render_partial_with_hyphen_and_invalid_option_as + e = assert_raises(ArgumentError) { @view.render(:partial => "test/a-in", :as => 'a-in') } + assert_equal "The value (a-in) of the option `as` is not a valid Ruby identifier; " + + "make sure it starts with lowercase letter, " + "and is followed by any combination of letters, numbers and underscores.", e.message end diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 8f2a069ba3..96e88f1b6c 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -99,7 +99,7 @@ module ActiveModel # user.authenticate('notright') # => false # user.authenticate('mUc3m00RsqyRe') # => user def authenticate(unencrypted_password) - BCrypt::Password.new(password_digest) == unencrypted_password && self + BCrypt::Password.new(password_digest).is_password?(unencrypted_password) && self end attr_reader :password diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index d9966f0ad7..c78ddd5fe6 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,31 @@ +* When a table has a composite primary key, the `primary_key` method for + SQLite3 and PostgreSQL adapters was only returning the first field of the key. + Ensures that it will return nil instead, as Active Record doesn't support + composite primary keys. + + Fixes #18070. + + *arthurnn* + +* `validates_size_of` / `validates_length_of` do not count records, + which are `marked_for_destruction?`. + + Fixes #7247. + + *Yves Senn* + +* Ensure `first!` and friends work on loaded associations. + + Fixes #18237. + + *Sean Griffin* + +* `eager_load` preserves readonly flag for associations. + + Closes #15853. + + *Takashi Kokubun* + * Provide `:touch` option to `save()` to accommodate saving without updating timestamps. diff --git a/activerecord/activerecord.gemspec b/activerecord/activerecord.gemspec index 6a00605ce1..c5cd0c89f7 100644 --- a/activerecord/activerecord.gemspec +++ b/activerecord/activerecord.gemspec @@ -24,5 +24,5 @@ Gem::Specification.new do |s| s.add_dependency 'activesupport', version s.add_dependency 'activemodel', version - s.add_dependency 'arel', '~> 6.0' + s.add_dependency 'arel', '7.0.0.alpha' end diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb index 0c3234ed24..f9c9f8afda 100644 --- a/activerecord/lib/active_record/associations/alias_tracker.rb +++ b/activerecord/lib/active_record/associations/alias_tracker.rb @@ -56,11 +56,11 @@ module ActiveRecord @connection = connection end - def aliased_table_for(table_name, aliased_name) + def aliased_table_for(table_name, aliased_name, **table_options) if aliases[table_name].zero? # If it's zero, we can have our table_name aliases[table_name] = 1 - Arel::Table.new(table_name) + Arel::Table.new(table_name, table_options) else # Otherwise, we need to use an alias aliased_name = connection.table_alias_for(aliased_name) @@ -73,7 +73,7 @@ module ActiveRecord else aliased_name end - Arel::Table.new(table_name).alias(table_alias) + Arel::Table.new(table_name, table_options).alias(table_alias) end end diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 0ac10531e5..53f65920e1 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -66,7 +66,8 @@ module ActiveRecord chain.map do |reflection| alias_tracker.aliased_table_for( table_name_for(reflection, klass, refl), - table_alias_for(reflection, refl, reflection != refl) + table_alias_for(reflection, refl, reflection != refl), + type_caster: klass.type_caster, ) end end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index e1c01cfe06..dc42b19a83 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -29,6 +29,7 @@ module ActiveRecord # instantiation of the actual post records. class CollectionProxy < Relation delegate(*(ActiveRecord::Calculations.public_instance_methods - [:count]), to: :scope) + delegate :find_nth, to: :scope def initialize(klass, association) #:nodoc: @association = association diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index cf63430a97..66e997c3c8 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -94,7 +94,7 @@ module ActiveRecord # def initialize(base, associations, joins) @alias_tracker = AliasTracker.create(base.connection, joins) - @alias_tracker.aliased_table_for(base.table_name, base.table_name) # Updates the count for base.table_name to 1 + @alias_tracker.aliased_table_for(base.table_name, base.table_name, type_caster: base.type_caster) # Updates the count for base.table_name to 1 tree = self.class.make_tree associations @join_root = JoinBase.new base, build(tree, base) @join_root.children.each { |child| construct_tables! @join_root, child } @@ -186,9 +186,13 @@ module ActiveRecord def table_aliases_for(parent, node) node.reflection.chain.map { |reflection| + if reflection.klass + type_caster = reflection.klass.type_caster + end alias_tracker.aliased_table_for( reflection.table_name, - table_alias_for(reflection, parent, reflection != node.reflection) + table_alias_for(reflection, parent, reflection != node.reflection), + type_caster: type_caster, ) } end @@ -257,6 +261,7 @@ module ActiveRecord construct(model, node, row, rs, seen, model_cache, aliases) else model = construct_model(ar_parent, node, row, model_cache, id, aliases) + model.readonly! seen[parent.base_klass][primary_id][node.base_klass][id] = model construct(model, node, row, rs, seen, model_cache, aliases) end diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 08f274fd42..aafb990bc1 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -122,6 +122,7 @@ module ActiveRecord end def clear_caches_calculated_from_columns + @arel_table = nil @attributes_builder = nil @column_names = nil @column_types = nil diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 954d22f1d5..bb01231bca 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -22,6 +22,7 @@ require 'active_record/log_subscriber' require 'active_record/explain_subscriber' require 'active_record/relation/delegation' require 'active_record/attributes' +require 'active_record/type_caster' module ActiveRecord #:nodoc: # = Active Record diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 74b0833a7e..b2a7b40ded 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -385,15 +385,15 @@ module ActiveRecord # Returns just a table's primary key def primary_key(table) - row = exec_query(<<-end_sql, 'SCHEMA').rows.first + pks = exec_query(<<-end_sql, 'SCHEMA').rows SELECT attr.attname FROM pg_attribute attr - INNER JOIN pg_constraint cons ON attr.attrelid = cons.conrelid AND attr.attnum = cons.conkey[1] + INNER JOIN pg_constraint cons ON attr.attrelid = cons.conrelid AND attr.attnum = any(cons.conkey) WHERE cons.contype = 'p' AND cons.conrelid = '#{quote_table_name(table)}'::regclass end_sql - - row && row.first + return nil unless pks.count == 1 + pks[0][0] end # Renames a table. diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 0f7e0fac01..67b2c6b1e6 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -418,10 +418,9 @@ module ActiveRecord end def primary_key(table_name) #:nodoc: - column = table_structure(table_name).find { |field| - field['pk'] == 1 - } - column && column['name'] + pks = table_structure(table_name).select { |f| f['pk'] > 0 } + return nil unless pks.count == 1 + pks[0]['name'] end def remove_index!(table_name, index_name) #:nodoc: diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index cb53fb0d44..38b2d632d2 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -235,7 +235,7 @@ module ActiveRecord # scope :published_and_commented, -> { published.and(self.arel_table[:comments_count].gt(0)) } # end def arel_table # :nodoc: - @arel_table ||= Arel::Table.new(table_name) + @arel_table ||= Arel::Table.new(table_name, type_caster: type_caster) end # Returns the Arel engine. @@ -252,6 +252,10 @@ module ActiveRecord @predicate_builder ||= PredicateBuilder.new(table_metadata) end + def type_caster # :nodoc: + TypeCaster::Map.new(self) + end + private def relation # :nodoc: diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 8a532402ba..b91e9ac137 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -193,7 +193,6 @@ module ActiveRecord def type_condition(table = arel_table) sti_column = table[inheritance_column] sti_names = ([self] + descendants).map(&:sti_name) - sti_names.map! { |v| Arel::Nodes::Quoted.new(v) } # FIXME: Remove this when type casting in Arel is removed (5.1) sti_column.in(sti_names) end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 7bca38c910..9f053453bd 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -11,7 +11,7 @@ module ActiveRecord # # == Usage # - # Active Records support optimistic locking if the field +lock_version+ is present. Each update to the + # Active Record supports optimistic locking if the +lock_version+ field is present. Each update to the # record increments the +lock_version+ column and the locking facilities ensure that records instantiated twice # will let the last one saved raise a +StaleObjectError+ if the first was also updated. Example: # diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index d76dbb43d6..641512d323 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -298,6 +298,7 @@ module ActiveRecord connection.schema_cache.clear_table_cache!(table_name) if table_exists? @arel_engine = nil + @arel_table = nil @column_names = nil @column_types = nil @content_columns = nil diff --git a/activerecord/lib/active_record/no_touching.rb b/activerecord/lib/active_record/no_touching.rb index dbf4564ae5..edb5066fa0 100644 --- a/activerecord/lib/active_record/no_touching.rb +++ b/activerecord/lib/active_record/no_touching.rb @@ -45,7 +45,7 @@ module ActiveRecord NoTouching.applied_to?(self.class) end - def touch(*) + def touch(*) # :nodoc: super unless no_touching? end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index ded190c111..f53c5f17ef 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -109,6 +109,10 @@ module ActiveRecord # validate: false, validations are bypassed altogether. See # ActiveRecord::Validations for more information. # + # By default, #save also sets the +updated_at+/+updated_on+ attributes to + # the current time. However, if you supply <tt>touch: false</tt>, these + # timestamps will not be updated. + # # There's a series of callbacks associated with +save+. If any of the # <tt>before_*</tt> callbacks return +false+ the action is cancelled and # +save+ returns +false+. See ActiveRecord::Callbacks for further @@ -131,6 +135,10 @@ module ActiveRecord # ActiveRecord::RecordInvalid gets raised. See ActiveRecord::Validations # for more information. # + # By default, #save! also sets the +updated_at+/+updated_on+ attributes to + # the current time. However, if you supply <tt>touch: false</tt>, these + # timestamps will not be updated. + # # There's a series of callbacks associated with <tt>save!</tt>. If any of # the <tt>before_*</tt> callbacks return +false+ the action is cancelled # and <tt>save!</tt> raises ActiveRecord::RecordNotSaved. See diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index f7b2167ae8..4f0502ae75 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -52,12 +52,7 @@ module ActiveRecord end else enum_for :find_each, options do - # FIXME: Remove this when type casting is removed from Arel - # (Rails 5.1). We can pass start directly instead. - if options[:start] - quoted_start = Arel::Nodes::Quoted.new(options[:start]) - end - options[:start] ? where(table[primary_key].gteq(quoted_start)).size : size + options[:start] ? where(table[primary_key].gteq(options[:start])).size : size end end end @@ -107,15 +102,9 @@ module ActiveRecord start = options[:start] batch_size = options[:batch_size] || 1000 - if start - # FIXME: Remove this when type casting is removed from Arel - # (Rails 5.1). We can pass start directly instead. - quoted_start = Arel::Nodes::Quoted.new(start) - end - unless block_given? return to_enum(:find_in_batches, options) do - total = start ? where(table[primary_key].gteq(quoted_start)).size : size + total = start ? where(table[primary_key].gteq(start)).size : size (total - 1).div(batch_size) + 1 end end @@ -125,7 +114,7 @@ module ActiveRecord end relation = relation.reorder(batch_order).limit(batch_size) - records = start ? relation.where(table[primary_key].gteq(quoted_start)).to_a : relation.to_a + records = start ? relation.where(table[primary_key].gteq(start)).to_a : relation.to_a while records.any? records_size = records.size @@ -136,11 +125,7 @@ module ActiveRecord break if records_size < batch_size - # FIXME: Remove this when type casting is removed from Arel - # (Rails 5.1). We can pass the offset directly instead. - quoted_offset = Arel::Nodes::Quoted.new(primary_key_offset) - - records = relation.where(table[primary_key].gt(quoted_offset)).to_a + records = relation.where(table[primary_key].gt(primary_key_offset)).to_a end end diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index ad8dddb9a4..567efce8ae 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -8,7 +8,7 @@ module ActiveRecord require 'active_record/relation/predicate_builder/range_handler' require 'active_record/relation/predicate_builder/relation_handler' - delegate :resolve_column_aliases, :type_cast_for_database, to: :table + delegate :resolve_column_aliases, to: :table def initialize(table) @table = table diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb index 7c90563d96..4b5f5773a0 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb @@ -19,12 +19,7 @@ module ActiveRecord case values.length when 0 then NullPredicate when 1 then predicate_builder.build(attribute, values.first) - else - attribute_name = attribute.name - casted_values = values.map do |v| - predicate_builder.type_cast_for_database(attribute_name, v) - end - attribute.in(casted_values) + else attribute.in(values) end unless nils.empty? diff --git a/activerecord/lib/active_record/relation/predicate_builder/basic_object_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/basic_object_handler.rb index 57a8b63001..6cec75dc0a 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/basic_object_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/basic_object_handler.rb @@ -6,7 +6,6 @@ module ActiveRecord end def call(attribute, value) - value = predicate_builder.type_cast_for_database(attribute.name, value) attribute.eq(value) end diff --git a/activerecord/lib/active_record/relation/predicate_builder/range_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/range_handler.rb index a6638738fa..1b3849e3ad 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/range_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/range_handler.rb @@ -6,11 +6,6 @@ module ActiveRecord end def call(attribute, value) - value = QuotedRange.new( - predicate_builder.type_cast_for_database(attribute.name, value.begin), - predicate_builder.type_cast_for_database(attribute.name, value.end), - value.exclude_end?, - ) attribute.between(value) end @@ -18,16 +13,5 @@ module ActiveRecord attr_reader :predicate_builder end - - class QuotedRange # :nodoc: - attr_reader :begin, :end, :exclude_end - alias_method :exclude_end?, :exclude_end - - def initialize(begin_val, end_val, exclude) - @begin = begin_val - @end = end_val - @exclude_end = exclude - end - end end end diff --git a/activerecord/lib/active_record/table_metadata.rb b/activerecord/lib/active_record/table_metadata.rb index e60bf55021..11e33e8dfe 100644 --- a/activerecord/lib/active_record/table_metadata.rb +++ b/activerecord/lib/active_record/table_metadata.rb @@ -8,12 +8,6 @@ module ActiveRecord @association = association end - def type_cast_for_database(attribute_name, value) - return value if value.is_a?(Arel::Nodes::BindParam) || klass.nil? - type = klass.type_for_attribute(attribute_name.to_s) - Arel::Nodes::Quoted.new(type.type_cast_for_database(value)) - end - def resolve_column_aliases(hash) hash = hash.dup hash.keys.grep(Symbol) do |key| @@ -35,17 +29,17 @@ module ActiveRecord def associated_table(table_name) return self if table_name == arel_table.name - arel_table = Arel::Table.new(table_name) association = klass._reflect_on_association(table_name) if association && !association.polymorphic? association_klass = association.klass - end - - if association - TableMetadata.new(association_klass, arel_table, association) + arel_table = association_klass.arel_table else - ConnectionAdapterTable.new(klass.connection, arel_table) + type_caster = TypeCaster::Connection.new(klass.connection, table_name) + association_klass = nil + arel_table = Arel::Table.new(table_name, type_caster: type_caster) end + + TableMetadata.new(association_klass, arel_table, association) end def polymorphic_association? @@ -56,65 +50,4 @@ module ActiveRecord attr_reader :klass, :arel_table, :association end - - # FIXME: We want to get rid of this class. The connection adapter does not - # have sufficient knowledge about types, as they could be provided by or - # overriden by the ActiveRecord::Base subclass. The case where you reach this - # class is if you do a query like: - # - # Liquid.joins(molecules: :electrons) - # .where("molecules.name" => "something", "electrons.name" => "something") - # - # Since we don't know that we can get to electrons through molecules - class ConnectionAdapterTable # :nodoc: - def initialize(connection, arel_table) - @connection = connection - @arel_table = arel_table - end - - def type_cast_for_database(attribute_name, value) - return value if value.is_a?(Arel::Nodes::BindParam) - type = type_for(attribute_name) - Arel::Nodes::Quoted.new(type.type_cast_for_database(value)) - end - - def resolve_column_aliases(hash) - hash - end - - def arel_attribute(column_name) - arel_table[column_name] - end - - def associated_with?(*) - false - end - - def associated_table(table_name) - arel_table = Arel::Table.new(table_name) - ConnectionAdapterTable.new(klass.connection, arel_table) - end - - def polymorphic_association? - false - end - - protected - - attr_reader :connection, :arel_table - - private - - def type_for(attribute_name) - if connection.schema_cache.table_exists?(arel_table.name) - column_for(attribute_name).cast_type - else - Type::Value.new - end - end - - def column_for(attribute_name) - connection.schema_cache.columns_hash(arel_table.name)[attribute_name.to_s] - end - end end diff --git a/activerecord/lib/active_record/type_caster.rb b/activerecord/lib/active_record/type_caster.rb new file mode 100644 index 0000000000..63ba10c289 --- /dev/null +++ b/activerecord/lib/active_record/type_caster.rb @@ -0,0 +1,7 @@ +require 'active_record/type_caster/map' +require 'active_record/type_caster/connection' + +module ActiveRecord + module TypeCaster + end +end diff --git a/activerecord/lib/active_record/type_caster/connection.rb b/activerecord/lib/active_record/type_caster/connection.rb new file mode 100644 index 0000000000..9e4a130b40 --- /dev/null +++ b/activerecord/lib/active_record/type_caster/connection.rb @@ -0,0 +1,34 @@ +module ActiveRecord + module TypeCaster + class Connection + def initialize(connection, table_name) + @connection = connection + @table_name = table_name + end + + def type_cast_for_database(attribute_name, value) + return value if value.is_a?(Arel::Nodes::BindParam) + type = type_for(attribute_name) + type.type_cast_for_database(value) + end + + protected + + attr_reader :connection, :table_name + + private + + def type_for(attribute_name) + if connection.schema_cache.table_exists?(table_name) + column_for(attribute_name).cast_type + else + Type::Value.new + end + end + + def column_for(attribute_name) + connection.schema_cache.columns_hash(table_name)[attribute_name.to_s] + end + end + end +end diff --git a/activerecord/lib/active_record/type_caster/map.rb b/activerecord/lib/active_record/type_caster/map.rb new file mode 100644 index 0000000000..03c9e8ff83 --- /dev/null +++ b/activerecord/lib/active_record/type_caster/map.rb @@ -0,0 +1,19 @@ +module ActiveRecord + module TypeCaster + class Map + def initialize(types) + @types = types + end + + def type_cast_for_database(attr_name, value) + return value if value.is_a?(Arel::Nodes::BindParam) + type = types.type_for_attribute(attr_name.to_s) + type.type_cast_for_database(value) + end + + protected + + attr_reader :types + end + end +end diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index a6c8ff7f3a..f27adc9c40 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -88,3 +88,4 @@ end require "active_record/validations/associated" require "active_record/validations/uniqueness" require "active_record/validations/presence" +require "active_record/validations/length" diff --git a/activerecord/lib/active_record/validations/length.rb b/activerecord/lib/active_record/validations/length.rb new file mode 100644 index 0000000000..ef5a6cbbe7 --- /dev/null +++ b/activerecord/lib/active_record/validations/length.rb @@ -0,0 +1,21 @@ +module ActiveRecord + module Validations + class LengthValidator < ActiveModel::Validations::LengthValidator # :nodoc: + def validate_each(record, attribute, association_or_value) + if association_or_value.respond_to?(:loaded?) && association_or_value.loaded? + association_or_value = association_or_value.target.reject(&:marked_for_destruction?) + end + super + end + end + + module ClassMethods + # See <tt>ActiveModel::Validation::LengthValidator</tt> for more information. + def validates_length_of(*attr_names) + validates_with LengthValidator, _merge_attributes(attr_names) + end + + alias_method :validates_size_of, :validates_length_of + end + end +end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index c4ff2e3ef3..9ff2ad9c6b 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -64,7 +64,6 @@ module ActiveRecord value = value.to_s[0, column.limit] end - # FIXME: Remove this when type casting is removed from Arel (Rails 5.1) value = Arel::Nodes::Quoted.new(value) comparison = if !options[:case_sensitive] && value && column.text? diff --git a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb index 28106d3772..85db8f4614 100644 --- a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb @@ -99,6 +99,12 @@ module ActiveRecord end end + def test_composite_primary_key + with_example_table '`id` INT(11), `number` INT(11), foo INT(11), PRIMARY KEY (`id`, `number`)' do + assert_nil @conn.primary_key('ex') + end + end + def test_tinyint_integer_typecasting with_example_table '`status` TINYINT(4)' do insert(@conn, { 'status' => 2 }, 'ex') diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index c3c696b871..6bb2b26cd5 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -54,6 +54,12 @@ module ActiveRecord end end + def test_composite_primary_key + with_example_table 'id serial, number serial, PRIMARY KEY (id, number)' do + assert_nil @connection.primary_key('ex') + end + end + def test_primary_key_raises_error_if_table_not_found assert_raises(ActiveRecord::StatementInvalid) do @connection.primary_key('unobtainium') diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 9d09ff49c7..029663e7f4 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -405,6 +405,12 @@ module ActiveRecord end end + def test_composite_primary_key + with_example_table 'id integer, number integer, foo integer, PRIMARY KEY (id, number)' do + assert_nil @conn.primary_key('ex') + end + end + def test_supports_extensions assert_not @conn.supports_extensions?, 'does not support extensions' end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index db8fd92c1f..fdb437d11d 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1328,7 +1328,6 @@ class EagerAssociationTest < ActiveRecord::TestCase end test "eager-loading readonly association" do - skip "eager_load does not yet preserve readonly associations" # has-one firm = Firm.where(id: "1").eager_load(:readonly_account).first! assert firm.readonly_account.readonly? @@ -1340,6 +1339,10 @@ class EagerAssociationTest < ActiveRecord::TestCase # has-many :through david = Author.where(id: "1").eager_load(:readonly_comments).first! assert david.readonly_comments.first.readonly? + + # belongs_to + post = Post.where(id: "1").eager_load(:author).first! + assert post.author.readonly? end test "preloading a polymorphic association with references to the associated table" do diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index c6769edcbf..72963fd56c 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -264,6 +264,11 @@ class AssociationProxyTest < ActiveRecord::TestCase end end + test "first! works on loaded associations" do + david = authors(:david) + assert_equal david.posts.first, david.posts.reload.first! + end + def test_reset_unloads_target david = authors(:david) david.posts.reload diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index aa56df62fd..eb76ef6328 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -25,8 +25,8 @@ class RelationMergingTest < ActiveRecord::TestCase end def test_relation_merging_with_arel_equalities_keeps_last_equality - devs = Developer.where(Developer.arel_table[:salary].eq(Arel::Nodes::Quoted.new(80000))).merge( - Developer.where(Developer.arel_table[:salary].eq(Arel::Nodes::Quoted.new(9000))) + devs = Developer.where(Developer.arel_table[:salary].eq(80000)).merge( + Developer.where(Developer.arel_table[:salary].eq(9000)) ) assert_equal [developers(:poor_jamis)], devs.to_a end diff --git a/activerecord/test/cases/relation/where_chain_test.rb b/activerecord/test/cases/relation/where_chain_test.rb index 24f6f1d2ab..619055f1e7 100644 --- a/activerecord/test/cases/relation/where_chain_test.rb +++ b/activerecord/test/cases/relation/where_chain_test.rb @@ -24,7 +24,7 @@ module ActiveRecord end def test_not_null - expected = Post.arel_table[@name].not_eq(Arel::Nodes::Quoted.new(nil)) + expected = Post.arel_table[@name].not_eq(nil) relation = Post.where.not(title: nil) assert_equal([expected], relation.where_values) end @@ -36,14 +36,13 @@ module ActiveRecord end def test_not_in - values = %w[hello goodbye].map { |v| Arel::Nodes::Quoted.new(v) } - expected = Post.arel_table[@name].not_in(values) + expected = Post.arel_table[@name].not_in(%w[hello goodbye]) relation = Post.where.not(title: %w[hello goodbye]) assert_equal([expected], relation.where_values) end def test_association_not_eq - expected = Comment.arel_table[@name].not_eq(Arel::Nodes::Quoted.new('hello')) + expected = Comment.arel_table[@name].not_eq('hello') relation = Post.joins(:comments).where.not(comments: {title: 'hello'}) assert_equal(expected.to_sql, relation.where_values.first.to_sql) end @@ -91,10 +90,7 @@ module ActiveRecord def test_chaining_multiple relation = Post.where.not(author_id: [1, 2]).where.not(title: 'ruby on rails') - expected = Post.arel_table['author_id'].not_in([ - Arel::Nodes::Quoted.new(1), - Arel::Nodes::Quoted.new(2), - ]) + expected = Post.arel_table['author_id'].not_in([1, 2]) assert_equal(expected, relation.where_values[0]) value = relation.where_values[1] diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 0819b6b11a..f7cb471984 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -64,21 +64,21 @@ module ActiveRecord def test_has_values relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) - relation.where! relation.table[:id].eq(Arel::Nodes::Quoted.new(10)) + relation.where! relation.table[:id].eq(10) assert_equal({:id => 10}, relation.where_values_hash) end def test_values_wrong_table relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) - relation.where! Comment.arel_table[:id].eq(Arel::Nodes::Quoted.new(10)) + relation.where! Comment.arel_table[:id].eq(10) assert_equal({}, relation.where_values_hash) end def test_tree_is_not_traversed relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) # FIXME: Remove the Arel::Nodes::Quoted in Rails 5.1 - left = relation.table[:id].eq(Arel::Nodes::Quoted.new(10)) - right = relation.table[:id].eq(Arel::Nodes::Quoted.new(10)) + left = relation.table[:id].eq(10) + right = relation.table[:id].eq(10) combine = left.and right relation.where! combine assert_equal({}, relation.where_values_hash) @@ -104,7 +104,7 @@ module ActiveRecord def test_create_with_value_with_wheres relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) # FIXME: Remove the Arel::Nodes::Quoted in Rails 5.1 - relation.where! relation.table[:id].eq(Arel::Nodes::Quoted.new(10)) + relation.where! relation.table[:id].eq(10) relation.create_with_value = {:hello => 'world'} assert_equal({:hello => 'world', :id => 10}, relation.scope_for_create) end @@ -115,7 +115,7 @@ module ActiveRecord assert_equal({}, relation.scope_for_create) # FIXME: Remove the Arel::Nodes::Quoted in Rails 5.1 - relation.where! relation.table[:id].eq(Arel::Nodes::Quoted.new(10)) + relation.where! relation.table[:id].eq(10) assert_equal({}, relation.scope_for_create) relation.create_with_value = {:hello => 'world'} diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index d56f998622..0738df1b54 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -145,13 +145,11 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal expected_5, received_5 expected_6 = Developer.order('salary DESC').collect(&:name) - # FIXME: Remove the Arel::Nodes::Quoted in Rails 5.1 - received_6 = DeveloperOrderedBySalary.where(Developer.arel_table['name'].eq(Arel::Nodes::Quoted.new('David'))).unscope(where: :name).collect(&:name) + received_6 = DeveloperOrderedBySalary.where(Developer.arel_table['name'].eq('David')).unscope(where: :name).collect(&:name) assert_equal expected_6, received_6 expected_7 = Developer.order('salary DESC').collect(&:name) - # FIXME: Remove the Arel::Nodes::Quoted in Rails 5.1 - received_7 = DeveloperOrderedBySalary.where(Developer.arel_table[:name].eq(Arel::Nodes::Quoted.new('David'))).unscope(where: :name).collect(&:name) + received_7 = DeveloperOrderedBySalary.where(Developer.arel_table[:name].eq('David')).unscope(where: :name).collect(&:name) assert_equal expected_7, received_7 end diff --git a/activerecord/test/cases/validations/length_validation_test.rb b/activerecord/test/cases/validations/length_validation_test.rb index 4a92da38ce..2c0e282761 100644 --- a/activerecord/test/cases/validations/length_validation_test.rb +++ b/activerecord/test/cases/validations/length_validation_test.rb @@ -2,6 +2,7 @@ require "cases/helper" require 'models/owner' require 'models/pet' +require 'models/person' class LengthValidationTest < ActiveRecord::TestCase fixtures :owners @@ -44,4 +45,21 @@ class LengthValidationTest < ActiveRecord::TestCase assert o.valid? end end + + def test_validates_size_of_reprects_records_marked_for_destruction + assert_nothing_raised { Owner.validates_size_of :pets, minimum: 1 } + owner = Owner.new + assert_not owner.save + assert owner.errors[:pets].any? + pet = owner.pets.build + assert owner.valid? + assert owner.save + + pet_count = Pet.count + assert_not owner.update_attributes pets_attributes: [ {_destroy: 1, id: pet.id} ] + assert_not owner.valid? + assert owner.errors[:pets].any? + assert_equal pet_count, Pet.count + end + end diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 9b740e405a..8c1f14bd36 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -34,7 +34,7 @@ class Author < ActiveRecord::Base -> { where(title: 'Welcome to the weblog').where('comments_count = ?', 1) }, class_name: 'Post' has_many :welcome_posts_with_comments, - -> { where(title: 'Welcome to the weblog').where(Post.arel_table[:comments_count].gt(Arel::Nodes::Quoted.new(0))) }, + -> { where(title: 'Welcome to the weblog').where(Post.arel_table[:comments_count].gt(0)) }, class_name: 'Post' has_many :comments_desc, -> { order('comments.id DESC') }, :through => :posts, :source => :comments diff --git a/activerecord/test/models/owner.rb b/activerecord/test/models/owner.rb index 2e3a9a3681..cedb774b10 100644 --- a/activerecord/test/models/owner.rb +++ b/activerecord/test/models/owner.rb @@ -17,6 +17,8 @@ class Owner < ActiveRecord::Base after_commit :execute_blocks + accepts_nested_attributes_for :pets, allow_destroy: true + def blocks @blocks ||= [] end diff --git a/activesupport/lib/active_support/core_ext/kernel/debugger.rb b/activesupport/lib/active_support/core_ext/kernel/debugger.rb index 2073cac98d..d8bb461010 100644 --- a/activesupport/lib/active_support/core_ext/kernel/debugger.rb +++ b/activesupport/lib/active_support/core_ext/kernel/debugger.rb @@ -1,6 +1,6 @@ module Kernel unless respond_to?(:debugger) - # Starts a debugging session if the +debugger+ gem has been loaded (call rails server --debugger to do load it). + # Starts a debugging session if the +debugger+ gem has been loaded (call rails server --debugger to load it). def debugger message = "\n***** Debugger requested, but was not available (ensure the debugger gem is listed in Gemfile/installed as gem): Start server with --debugger to enable *****\n" defined?(Rails) ? Rails.logger.info(message) : $stderr.puts(message) diff --git a/activesupport/lib/active_support/core_ext/object.rb b/activesupport/lib/active_support/core_ext/object.rb index f1106cca9b..f4f9152d6a 100644 --- a/activesupport/lib/active_support/core_ext/object.rb +++ b/activesupport/lib/active_support/core_ext/object.rb @@ -2,7 +2,6 @@ require 'active_support/core_ext/object/acts_like' require 'active_support/core_ext/object/blank' require 'active_support/core_ext/object/duplicable' require 'active_support/core_ext/object/deep_dup' -require 'active_support/core_ext/object/itself' require 'active_support/core_ext/object/try' require 'active_support/core_ext/object/inclusion' diff --git a/activesupport/lib/active_support/core_ext/object/itself.rb b/activesupport/lib/active_support/core_ext/object/itself.rb deleted file mode 100644 index d71cea6674..0000000000 --- a/activesupport/lib/active_support/core_ext/object/itself.rb +++ /dev/null @@ -1,15 +0,0 @@ -class Object - # TODO: Remove this file when we drop support for Ruby < 2.2 - unless respond_to?(:itself) - # Returns the object itself. - # - # Useful for chaining methods, such as Active Record scopes: - # - # Event.public_send(state.presence_in([ :trashed, :drafted ]) || :itself).order(:created_at) - # - # @return Object - def itself - self - end - end -end diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb index ba92afd5f4..231eaedbba 100644 --- a/activesupport/lib/active_support/core_ext/string/output_safety.rb +++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb @@ -150,7 +150,11 @@ module ActiveSupport #:nodoc: else if html_safe? new_safe_buffer = super - new_safe_buffer.instance_variable_set :@html_safe, true + + if new_safe_buffer + new_safe_buffer.instance_variable_set :@html_safe, true + end + new_safe_buffer else to_str[*args] diff --git a/activesupport/test/core_ext/object/itself_test.rb b/activesupport/test/core_ext/object/itself_test.rb deleted file mode 100644 index 65db0ddf40..0000000000 --- a/activesupport/test/core_ext/object/itself_test.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'abstract_unit' -require 'active_support/core_ext/object' - -class Object::ItselfTest < ActiveSupport::TestCase - test 'itself returns self' do - object = 'fun' - assert_equal object, object.itself - end -end diff --git a/activesupport/test/safe_buffer_test.rb b/activesupport/test/safe_buffer_test.rb index efa9d5e61f..4532152996 100644 --- a/activesupport/test/safe_buffer_test.rb +++ b/activesupport/test/safe_buffer_test.rb @@ -165,4 +165,9 @@ class SafeBufferTest < ActiveSupport::TestCase x = 'foo %{x} bar'.html_safe % { x: 'qux' } assert x.html_safe?, 'should be safe' end + + test 'Should not affect frozen objects when accessing characters' do + x = 'Hello'.html_safe + assert_equal x[/a/, 1], nil + end end diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md index c9e35dd985..c586675ee5 100644 --- a/guides/source/action_mailer_basics.md +++ b/guides/source/action_mailer_basics.md @@ -733,7 +733,9 @@ Mailer framework. You can do this in an initializer file `config/initializers/sandbox_email_interceptor.rb` ```ruby -ActionMailer::Base.register_interceptor(SandboxEmailInterceptor) if Rails.env.staging? +if Rails.env.staging? + ActionMailer::Base.register_interceptor(SandboxEmailInterceptor) +end ``` NOTE: The example above uses a custom environment called "staging" for a diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 217a64502b..0a375d7cb8 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -320,6 +320,8 @@ The schema dumper adds one additional configuration option: * `config.action_controller.default_charset` specifies the default character set for all renders. The default is "utf-8". +* `config.action_controller.include_all_helpers` configures whether all view helpers are available everywhere or are scoped to the corresponding controller. If set to `false`, `UsersHelper` methods are only available for views rendered as part of `UsersController`. If `true`, `UsersHelper` methods are available everywhere. The default is `true`. + * `config.action_controller.logger` accepts a logger conforming to the interface of Log4r or the default Ruby Logger class, which is then used to log information from Action Controller. Set to `nil` to disable logging. * `config.action_controller.request_forgery_protection_token` sets the token parameter name for RequestForgery. Calling `protect_from_forgery` sets it to `:authenticity_token` by default. diff --git a/guides/source/documents.yaml b/guides/source/documents.yaml index 8dd310d007..64b4c16221 100644 --- a/guides/source/documents.yaml +++ b/guides/source/documents.yaml @@ -86,8 +86,7 @@ - name: Testing Rails Applications url: testing.html - work_in_progress: true - description: This is a rather comprehensive guide to doing both unit and functional tests in Rails. It covers everything from 'What is a test?' to the testing APIs. Enjoy. + description: This is a rather comprehensive guide to the various testing facilities in Rails. It covers everything from 'What is a test?' to the testing APIs. Enjoy. - name: Securing Rails Applications url: security.html @@ -113,11 +112,6 @@ url: working_with_javascript_in_rails.html description: This guide covers the built-in Ajax/JavaScript functionality of Rails. - - name: Getting Started with Engines - url: engines.html - description: This guide explains how to write a mountable engine. - work_in_progress: true - - name: The Rails Initialization Process work_in_progress: true url: initialization.html @@ -147,6 +141,11 @@ name: Creating and Customizing Rails Generators url: generators.html description: This guide covers the process of adding a brand new generator to your extension or providing an alternative to an element of a built-in Rails generator (such as providing alternative test stubs for the scaffold generator). + - + name: Getting Started with Engines + url: engines.html + description: This guide explains how to write a mountable engine. + work_in_progress: true - name: Contributing to Ruby on Rails documents: diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 36947d086a..ff6e0db480 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -2034,9 +2034,14 @@ What's Next? ------------ Now that you've seen your first Rails application, you should feel free to -update it and experiment on your own. But you don't have to do everything -without help. As you need assistance getting up and running with Rails, feel -free to consult these support resources: +update it and experiment on your own. + +We recommend next that you read [A Guide to Testing Rails Applications](testing.html), +for a deep dive into Rails testing facilities and approaches. + +Remember you don't have to do everything without help. As you need assistance +getting up and running with Rails, feel free to consult these support +resources: * The [Ruby on Rails Guides](index.html) * The [Ruby on Rails Tutorial](http://railstutorial.org/book) diff --git a/guides/source/testing.md b/guides/source/testing.md index f7103adc1c..2d66840306 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -141,6 +141,27 @@ users(:david).id email(david.girlfriend.email, david.location_tonight) ``` +### Rake Tasks for Running your Tests + +Rails comes with a number of built-in rake tasks to help with testing. The +table below lists the commands included in the default Rakefile when a Rails +project is created. + +| Tasks | Description | +| ----------------------- | ----------- | +| `rake test` | Runs all tests in the `test` directory. You can also run `rake` and Rails will run all tests by default | +| `rake test:controllers` | Runs all the controller tests from `test/controllers` | +| `rake test:functionals` | Runs all the functional tests from `test/controllers`, `test/mailers`, and `test/functional` | +| `rake test:helpers` | Runs all the helper tests from `test/helpers` | +| `rake test:integration` | Runs all the integration tests from `test/integration` | +| `rake test:jobs` | Runs all the job tests from `test/jobs` | +| `rake test:mailers` | Runs all the mailer tests from `test/mailers` | +| `rake test:models` | Runs all the model tests from `test/models` | +| `rake test:units` | Runs all the unit tests from `test/models`, `test/helpers`, and `test/unit` | +| `rake test:db` | Runs all tests in the `test` directory and resets the db | + +We will cover each of types Rails tests listed above in this guide. + Unit Testing your Models ------------------------ @@ -388,44 +409,9 @@ This test should now pass. By now you've caught a glimpse of some of the assertions that are available. Assertions are the worker bees of testing. They are the ones that actually perform the checks to ensure that things are going as planned. -There are a bunch of different types of assertions you can use. -Here's an extract of the assertions you can use with [`Minitest`](https://github.com/seattlerb/minitest), the default testing library used by Rails. The `[msg]` parameter is an optional string message you can specify to make your test failure messages clearer. It's not required. - -| Assertion | Purpose | -| ---------------------------------------------------------------- | ------- | -| `assert( test, [msg] )` | Ensures that `test` is true.| -| `assert_not( test, [msg] )` | Ensures that `test` is false.| -| `assert_equal( expected, actual, [msg] )` | Ensures that `expected == actual` is true.| -| `assert_not_equal( expected, actual, [msg] )` | Ensures that `expected != actual` is true.| -| `assert_same( expected, actual, [msg] )` | Ensures that `expected.equal?(actual)` is true.| -| `assert_not_same( expected, actual, [msg] )` | Ensures that `expected.equal?(actual)` is false.| -| `assert_nil( obj, [msg] )` | Ensures that `obj.nil?` is true.| -| `assert_not_nil( obj, [msg] )` | Ensures that `obj.nil?` is false.| -| `assert_empty( obj, [msg] )` | Ensures that `obj` is `empty?`.| -| `assert_not_empty( obj, [msg] )` | Ensures that `obj` is not `empty?`.| -| `assert_match( regexp, string, [msg] )` | Ensures that a string matches the regular expression.| -| `assert_no_match( regexp, string, [msg] )` | Ensures that a string doesn't match the regular expression.| -| `assert_includes( collection, obj, [msg] )` | Ensures that `obj` is in `collection`.| -| `assert_not_includes( collection, obj, [msg] )` | Ensures that `obj` is not in `collection`.| -| `assert_in_delta( expecting, actual, [delta], [msg] )` | Ensures that the numbers `expected` and `actual` are within `delta` of each other.| -| `assert_not_in_delta( expecting, actual, [delta], [msg] )` | Ensures that the numbers `expected` and `actual` are not within `delta` of each other.| -| `assert_throws( symbol, [msg] ) { block }` | Ensures that the given block throws the symbol.| -| `assert_raises( exception1, exception2, ... ) { block }` | Ensures that the given block raises one of the given exceptions.| -| `assert_nothing_raised( exception1, exception2, ... ) { block }` | Ensures that the given block doesn't raise one of the given exceptions.| -| `assert_instance_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class`.| -| `assert_not_instance_of( class, obj, [msg] )` | Ensures that `obj` is not an instance of `class`.| -| `assert_kind_of( class, obj, [msg] )` | Ensures that `obj` is or descends from `class`.| -| `assert_not_kind_of( class, obj, [msg] )` | Ensures that `obj` is not an instance of `class` and is not descending from it.| -| `assert_respond_to( obj, symbol, [msg] )` | Ensures that `obj` responds to `symbol`.| -| `assert_not_respond_to( obj, symbol, [msg] )` | Ensures that `obj` does not respond to `symbol`.| -| `assert_operator( obj1, operator, [obj2], [msg] )` | Ensures that `obj1.operator(obj2)` is true.| -| `assert_not_operator( obj1, operator, [obj2], [msg] )` | Ensures that `obj1.operator(obj2)` is false.| -| `assert_predicate ( obj, predicate, [msg] )` | Ensures that `obj.predicate` is true, e.g. `assert_predicate str, :empty?`| -| `assert_not_predicate ( obj, predicate, [msg] )` | Ensures that `obj.predicate` is false, e.g. `assert_not_predicate str, :empty?`| -| `assert_send( array, [msg] )` | Ensures that executing the method listed in `array[1]` on the object in `array[0]` with the parameters of `array[2 and up]` is true. This one is weird eh?| -| `flunk( [msg] )` | Ensures failure. This is useful to explicitly mark a test that isn't finished yet.| - -The above are subset of assertions that minitest supports. For an exhaustive & more up-to-date list, please check [Minitest API documentation](http://docs.seattlerb.org/minitest/), specifically [`Minitest::Assertions`](http://docs.seattlerb.org/minitest/Minitest/Assertions.html) +There are a bunch of different types of assertions you can use that come with [`Minitest`](https://github.com/seattlerb/minitest), the default testing library used by Rails. + +For a list of all available assertions please check the [Minitest API documentation](http://docs.seattlerb.org/minitest/), specifically [`Minitest::Assertions`](http://docs.seattlerb.org/minitest/Minitest/Assertions.html) Because of the modular nature of the testing framework, it is possible to create your own assertions. In fact, that's exactly what Rails does. It includes some specialized assertions to make your life easier. @@ -447,10 +433,24 @@ Rails adds some custom assertions of its own to the `minitest` framework: You'll see the usage of some of these assertions in the next chapter. +### A Brief Note About Minitest + +All the basic assertions such as `assert_equal` defined in `Minitest::Assertions` are also available in the classes we use in our own test cases. In fact, Rails provides the following classes for you to inherit from: + +* `ActiveSupport::TestCase` +* `ActionController::TestCase` +* `ActionMailer::TestCase` +* `ActionView::TestCase` +* `ActionDispatch::IntegrationTest` + +Each of these classes include `Minitest::Assertions`, allowing us to use all of the basic assertions in our tests. + +NOTE: For more information on `Minitest`, refer to [Minitest](http://ruby-doc.org/stdlib-2.1.0/libdoc/minitest/rdoc/MiniTest.html) + Functional Tests for Your Controllers ------------------------------------- -In Rails, testing the various actions of a single controller is called writing functional tests for that controller. Controllers handle the incoming web requests to your application and eventually respond with a rendered view. +In Rails, testing the various actions of a controller is a form of writing functional tests. Remember your controllers handle the incoming web requests to your application and eventually respond with a rendered view. When writing functional tests, you're testing how your actions handle the requests and the expected result, or response in some cases an HTML view. ### What to Include in your Functional Tests @@ -524,13 +524,13 @@ If you're familiar with the HTTP protocol, you'll know that `get` is a type of r * `head` * `delete` -All of request types are methods that you can use, however, you'll probably end up using the first two more often than the others. +All of request types have equivalent methods that you can use. In a typical C.R.U.D. application you'll be using `get`, `post`, `put` and `delete` more often. -NOTE: Functional tests do not verify whether the specified request type should be accepted by the action. Request types in this context exist to make your tests more descriptive. +NOTE: Functional tests do not verify whether the specified request type is accepted by the action, we're more concerned with the result. Request tests exist for this use case to make your tests more purposeful. ### The Four Hashes of the Apocalypse -After a request has been made using one of the 6 methods (`get`, `post`, etc.) and processed, you will have 4 Hash objects ready for use: +After a request has been made and processed, you will have 4 Hash objects ready for use: * `assigns` - Any objects that are stored as instance variables in actions for use in views. * `cookies` - Any cookies that are set. @@ -553,8 +553,8 @@ assigns["something"] assigns(:something) You also have access to three instance variables in your functional tests: * `@controller` - The controller processing the request -* `@request` - The request -* `@response` - The response +* `@request` - The request object +* `@response` - The response object ### Setting Headers and CGI variables @@ -575,6 +575,10 @@ post :create # simulate the request with custom env variable ### Testing Templates and Layouts +Eventually, you may want to test whether a specific layout is rendered in the view of a response. + +#### Asserting Templates + If you want to make sure that the response rendered the correct template and layout, you can use the `assert_template` method: @@ -583,24 +587,22 @@ test "index should render correct template and layout" do get :index assert_template :index assert_template layout: "layouts/application" + + # You can also pass a regular expression. + assert_template layout: /layouts\/application/ end ``` -Note that you cannot test for template and layout at the same time, with one call to `assert_template` method. -Also, for the `layout` test, you can give a regular expression instead of a string, but using the string, makes -things clearer. On the other hand, you have to include the "layouts" directory name even if you save your layout -file in this standard layout directory. Hence, +NOTE: You cannot test for template and layout at the same time, with a single call to `assert_template`. -```ruby -assert_template layout: "application" -``` +WARNING: You must include the "layouts" directory name even if you save your layout file in this standard layout directory. Hence, `assert_template layout: "application"` will not work. -will not work. +#### Asserting Partials -If your view renders any partial, when asserting for the layout, you have to assert for the partial at the same time. +If your view renders any partial, when asserting for the layout, you can to assert for the partial at the same time. Otherwise, assertion will fail. -Hence: +Remember, we added the "_form" partial to our creating Articles view? Let's write an assertion for that in the `:new` action now: ```ruby test "new should render correct layout" do @@ -609,27 +611,201 @@ test "new should render correct layout" do end ``` -is the correct way to assert for the layout when the view renders a partial with name `_form`. Omitting the `:partial` key in your `assert_template` call will complain. +This is the correct way to assert for when the view renders a partial with a given name. As identified by the `:partial` key passed to the `assert_template` call. -### A Fuller Functional Test Example +### Testing `flash` notices -Here's another example that uses `flash`, `assert_redirected_to`, and `assert_difference`: +If you remember from earlier one of the Four Hashes of the Apocalypse was `flash`. + +We want to add a `flash` message to our blog application whenever someone +successfully creates a new Article. + +Let's start by adding this assertion to our `test_should_create_article` test: ```ruby test "should create article" do assert_difference('Article.count') do - post :create, article: {title: 'Hi', body: 'This is my first article.'} + post :create, article: {title: 'Some title'} end + assert_redirected_to article_path(assigns(:article)) assert_equal 'Article was successfully created.', flash[:notice] end ``` -### Testing Views +If we run our test now, we should see a failure: + +```bash +$ bin/rake test test/controllers/articles_controller_test.rb test_should_create_article +Run options: -n test_should_create_article --seed 32266 + +# Running: + +F + +Finished in 0.114870s, 8.7055 runs/s, 34.8220 assertions/s. + + 1) Failure: +ArticlesControllerTest#test_should_create_article [/Users/zzak/code/bench/sharedapp/test/controllers/articles_controller_test.rb:16]: +--- expected ++++ actual +@@ -1 +1 @@ +-"Article was successfully created." ++nil + +1 runs, 4 assertions, 1 failures, 0 errors, 0 skips +``` + +Let's implement the flash message now in our controller. Our `:create` action should now look like this: + +```ruby +def create + @article = Article.new(article_params) + + if @article.save + flash[:notice] = 'Article was successfully created.' + redirect_to @article + else + render 'new' + end +end +``` + +Now if we run our tests, we should see it pass: -Testing the response to your request by asserting the presence of key HTML elements and their content is a useful way to test the views of your application. The `assert_select` assertion allows you to do this by using a simple yet powerful syntax. +```bash +$ bin/rake test test/controllers/articles_controller_test.rb test_should_create_article +Run options: -n test_should_create_article --seed 18981 -NOTE: You may find references to `assert_tag` in other documentation. This has been removed in 4.2. Use `assert_select` instead. +# Running: + +. + +Finished in 0.081972s, 12.1993 runs/s, 48.7972 assertions/s. + +1 runs, 4 assertions, 0 failures, 0 errors, 0 skips +``` + +### Putting it together + +At this point our Articles controller tests the `:index` as well as `:new` and `:create` actions. What about dealing with existing data? + +Let's write a test for the `:show` action: + +```ruby +test "should show article" do + article = articles(:one) + get :show, id: article.id + assert_response :success +end +``` + +Remember from our discussion earlier on fixtures the `articles()` method will give us access to our Articles fixtures. + +How about deleting an existing Article? + +```ruby +test "should destroy article" do + article = articles(:one) + assert_difference('Article.count', -1) do + delete :destroy, id: article.id + end + + assert_redirected_to articles_path +end +``` + +We can also add a test for updating an existing Article. + +```ruby +test "should update article" do + article = articles(:one) + patch :update, id: article.id, article: {title: "updated"} + assert_redirected_to article_path(assigns(:article)) +end +``` + +Notice we're starting to see some duplication in these three tests, they both access the same Article fixture data. We can D.R.Y. this up by using the `setup` and `teardown` methods provided by `ActiveSupport::Callbacks`. + +Our test should now look something like this, disregard the other tests we're leaving them out for brevity. + +```ruby +require 'test_helper' + +class ArticlesControllerTest < ActionController::TestCase + # called before every single test + def setup + @article = articles(:one) + end + + # called after every single test + def teardown + # as we are re-initializing @article before every test + # setting it to nil here is not essential but I hope + # you understand how you can use the teardown method + @article = nil + end + + test "should show article" do + # Reuse the @article instance variable from setup + get :show, id: @article.id + assert_response :success + end + + test "should destroy article" do + assert_difference('Article.count', -1) do + delete :destroy, id: @article.id + end + + assert_redirected_to articles_path + end + + test "should update article" do + patch :update, id: @article.id, article: {title: "updated"} + assert_redirected_to article_path(assigns(:article)) + end +end +``` + +Similar to other callbacks in Rails, the `setup` and `teardown` methods can also be used by passing a block, lambda, or method name as a symbol to call. + +Testing Routes +-------------- + +Like everything else in your Rails application, it is recommended that you test your routes. Below are example tests for the routes of default `show` and `create` action of `Articles` controller above and it should look like: + +```ruby +class ArticleRoutesTest < ActionController::TestCase + test "should route to article" do + assert_routing '/articles/1', { controller: "articles", action: "show", id: "1" } + end + + test "should route to create article" do + assert_routing({ method: 'post', path: '/articles' }, { controller: "articles", action: "create" }) + end +end +``` + +I've added this file here `test/controllers/articles_routes_test.rb` and if we run the test we should see: + +```bash +$ be rake test test/controllers/articles_routes_test.rb + +# Running: + +.. + +Finished in 0.069381s, 28.8263 runs/s, 86.4790 assertions/s. + +2 runs, 6 assertions, 0 failures, 0 errors, 0 skips +``` + +For more information on routing assertions available in Rails, see the API documentation for [`ActionDispatch::Assertions::RoutingAssertions`](http://api.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html). + +Testing Views +------------- + +Testing the response to your request by asserting the presence of key HTML elements and their content is a common way to test the views of your application. The `assert_select` method allows you to query HTML elements of the response by using a simple yet powerful syntax. There are two forms of `assert_select`: @@ -643,7 +819,10 @@ For example, you could verify the contents on the title element in your response assert_select 'title', "Welcome to Rails Testing Guide" ``` -You can also use nested `assert_select` blocks. In this case the inner `assert_select` runs the assertion on the complete collection of elements selected by the outer `assert_select` block: +You can also use nested `assert_select` blocks for deeper investigation. + +In the following example, the inner `assert_select` for `li.menu_item` runs +within the collection of elements selected by the outer block: ```ruby assert_select 'ul.navigation' do @@ -651,7 +830,9 @@ assert_select 'ul.navigation' do end ``` -Alternatively the collection of elements selected by the outer `assert_select` may be iterated through so that `assert_select` may be called separately for each element. Suppose for example that the response contains two ordered lists, each with four list elements then the following tests will both pass. +A collection of selected elements may be iterated through so that `assert_select` may be called separately for each element. + +For example if the response contains two ordered lists, each with four nested list elements then the following tests will both pass. ```ruby assert_select "ol" do |elements| @@ -665,7 +846,7 @@ assert_select "ol" do end ``` -The `assert_select` assertion is quite powerful. For more advanced usage, refer to its [documentation](https://github.com/rails/rails-dom-testing/blob/master/lib/rails/dom/testing/assertions/selector_assertions.rb). +This assertion is quite powerful. For more advanced usage, refer to its [documentation](http://www.rubydoc.info/github/rails/rails-dom-testing). #### Additional View-Based Assertions @@ -685,12 +866,45 @@ assert_select_email do end ``` +Testing helpers +--------------- + +In order to test helpers, all you need to do is check that the output of the +helper method matches what you'd expect. Tests related to the helpers are +located under the `test/helpers` directory. + +A helper test looks like so: + +```ruby +require 'test_helper' + +class UserHelperTest < ActionView::TestCase +end +``` + +A helper is just a simple module where you can define methods which are +available into your views. To test the output of the helper's methods, you just +have to use a mixin like this: + +```ruby +class UserHelperTest < ActionView::TestCase + include UserHelper + + test "should return the user name" do + # ... + end +end +``` + +Moreover, since the test class extends from `ActionView::TestCase`, you have +access to Rails' helper methods such as `link_to` or `pluralize`. + Integration Testing ------------------- -Integration tests are used to test the interaction among any number of controllers. They are generally used to test important work flows within your application. +Integration tests are used to test how various parts of your application interact. They are generally used to test important work flows within your application. -Unlike Unit and Functional tests, integration tests have to be explicitly created under the 'test/integration' directory within your application. Rails provides a generator to create an integration test skeleton for you. +For creating Rails integration tests, we use the 'test/integration' directory for your application. Rails provides a generator to create an integration test skeleton for you. ```bash $ bin/rails generate integration_test user_flows @@ -710,233 +924,92 @@ class UserFlowsTest < ActionDispatch::IntegrationTest end ``` -Integration tests inherit from `ActionDispatch::IntegrationTest`. This makes available some additional helpers to use in your integration tests. Also you need to explicitly include the fixtures to be made available to the test. +Inheriting from `ActionDispatch::IntegrationTest` comes with some advantages. This makes available some additional helpers to use in your integration tests. ### Helpers Available for Integration Tests -In addition to the standard testing helpers, there are some additional helpers available to integration tests: +In addition to the standard testing helpers, inheriting `ActionDispatch::IntegrationTest` comes with some additional helpers available when writing integration tests. Let's briefly introduce you to the three categories of helpers you get to choose from. -| Helper | Purpose | -| ------------------------------------------------------------------ | ------- | -| `https?` | Returns `true` if the session is mimicking a secure HTTPS request.| -| `https!` | Allows you to mimic a secure HTTPS request.| -| `host!` | Allows you to set the host name to use in the next request.| -| `redirect?` | Returns `true` if the last request was a redirect.| -| `follow_redirect!` | Follows a single redirect response.| -| `request_via_redirect(http_method, path, [parameters], [headers])` | Allows you to make an HTTP request and follow any subsequent redirects.| -| `post_via_redirect(path, [parameters], [headers])` | Allows you to make an HTTP POST request and follow any subsequent redirects.| -| `get_via_redirect(path, [parameters], [headers])` | Allows you to make an HTTP GET request and follow any subsequent redirects.| -| `patch_via_redirect(path, [parameters], [headers])` | Allows you to make an HTTP PATCH request and follow any subsequent redirects.| -| `put_via_redirect(path, [parameters], [headers])` | Allows you to make an HTTP PUT request and follow any subsequent redirects.| -| `delete_via_redirect(path, [parameters], [headers])` | Allows you to make an HTTP DELETE request and follow any subsequent redirects.| -| `open_session` | Opens a new session instance.| +For dealing with the integration test runner, see [`ActionDispatch::Integration::Runner`](http://api.rubyonrails.org/classes/ActionDispatch/Integration/Runner.html). -### Integration Testing Examples +When performing requests, you will have [`ActionDispatch::Integration::RequestHelpers`](http://api.rubyonrails.org/classes/ActionDispatch/Integration/RequestHelpers.html) available for your use. -A simple integration test that exercises multiple controllers: +If you'd like to modify the session, or state of your integration test you should look for [`ActionDispatch::Integration::Session`](http://api.rubyonrails.org/classes/ActionDispatch/Integration/Session.html) to help. -```ruby -require 'test_helper' +### Implementing an integration test -class UserFlowsTest < ActionDispatch::IntegrationTest - test "login and browse site" do - # login via https - https! - get "/login" - assert_response :success +Let's add an integration test to our blog application. We'll start with a basic workflow of creating a new blog article, to verify that everything is working properly. - post_via_redirect "/login", username: users(:david).username, password: users(:david).password - assert_equal '/welcome', path - assert_equal 'Welcome david!', flash[:notice] +We'll start by generating our integration test skeleton: - https!(false) - get "/articles/all" - assert_response :success - assert assigns(:articles) - end -end +```bash +$ bin/rails generate integration_test blog_flow ``` -As you can see the integration test involves multiple controllers and exercises the entire stack from database to dispatcher. In addition you can have multiple session instances open simultaneously in a test and extend those instances with assertion methods to create a very powerful testing DSL (domain-specific language) just for your application. +It should have created a test file placeholder for us, with the output of the previous command you should see: + +```bash + invoke test_unit + create test/integration/blog_flow_test.rb +``` -Here's an example of multiple sessions and custom DSL in an integration test +Now let's open that file and write our first assertion: ```ruby require 'test_helper' -class UserFlowsTest < ActionDispatch::IntegrationTest - test "login and browse site" do - # User david logs in - david = login(:david) - # User guest logs in - guest = login(:guest) - - # Both are now available in different sessions - assert_equal 'Welcome david!', david.flash[:notice] - assert_equal 'Welcome guest!', guest.flash[:notice] - - # User david can browse site - david.browses_site - # User guest can browse site as well - guest.browses_site - - # Continue with other assertions +class BlogFlowTest < ActionDispatch::IntegrationTest + test "can see the welcome page" do + get "/" + assert_select "h1", "Welcome#index" end - - private - - module CustomDsl - def browses_site - get "/products/all" - assert_response :success - assert assigns(:products) - end - end - - def login(user) - open_session do |sess| - sess.extend(CustomDsl) - u = users(user) - sess.https! - sess.post "/login", username: u.username, password: u.password - assert_equal '/welcome', sess.path - sess.https!(false) - end - end end ``` -Rake Tasks for Running your Tests ---------------------------------- - -Rails comes with a number of built-in rake tasks to help with testing. The -table below lists the commands included in the default Rakefile when a Rails -project is created. - -| Tasks | Description | -| ----------------------- | ----------- | -| `rake test` | Runs all tests in the `test` directory. You can also run `rake` and Rails will run all tests by default | -| `rake test:controllers` | Runs all the controller tests from `test/controllers` | -| `rake test:functionals` | Runs all the functional tests from `test/controllers`, `test/mailers`, and `test/functional` | -| `rake test:helpers` | Runs all the helper tests from `test/helpers` | -| `rake test:integration` | Runs all the integration tests from `test/integration` | -| `rake test:jobs` | Runs all the job tests from `test/jobs` | -| `rake test:mailers` | Runs all the mailer tests from `test/mailers` | -| `rake test:models` | Runs all the model tests from `test/models` | -| `rake test:units` | Runs all the unit tests from `test/models`, `test/helpers`, and `test/unit` | -| `rake test:db` | Runs all tests in the `test` directory and resets the db | - +If you remember from earlier in the "Testing Views" section we covered `assert_select` to query the resulting HTML of a request. -A Brief Note About Minitest ------------------------------ +When visit our root path, we should see `welcome/index.html.erb` rendered for the view. So this assertion should pass. -Ruby ships with a vast Standard Library for all common use-cases including testing. Since version 1.9, Ruby provides `Minitest`, a framework for testing. All the basic assertions such as `assert_equal` discussed above are actually defined in `Minitest::Assertions`. The classes `ActiveSupport::TestCase`, `ActionController::TestCase`, `ActionMailer::TestCase`, `ActionView::TestCase` and `ActionDispatch::IntegrationTest` - which we have been inheriting in our test classes - include `Minitest::Assertions`, allowing us to use all of the basic assertions in our tests. +#### Creating articles integration -NOTE: For more information on `Minitest`, refer to [Minitest](http://ruby-doc.org/stdlib-2.1.0/libdoc/minitest/rdoc/MiniTest.html) - -Setup and Teardown ------------------- - -If you would like to run a block of code before the start of each test and another block of code after the end of each test you have two special callbacks for your rescue. Let's take note of this by looking at an example for our functional test in `Articles` controller: +How about testing our ability to create a new article in our blog and see the resulting article. ```ruby -require 'test_helper' - -class ArticlesControllerTest < ActionController::TestCase - - # called before every single test - def setup - @article = articles(:one) - end - - # called after every single test - def teardown - # as we are re-initializing @article before every test - # setting it to nil here is not essential but I hope - # you understand how you can use the teardown method - @article = nil - end - - test "should show article" do - get :show, id: @article.id - assert_response :success - end - - test "should destroy article" do - assert_difference('Article.count', -1) do - delete :destroy, id: @article.id - end - - assert_redirected_to articles_path - end - +test "can create an article" do + get "/articles/new" + assert_response :success + assert_template "articles/new", partial: "articles/_form" + + post "/articles", article: {title: "can create", body: "article successfully."} + assert_response :redirect + follow_redirect! + assert_response :success + assert_template "articles/show" + assert_select "p", "Title:\n can create" end ``` -Above, the `setup` method is called before each test and so `@article` is available for each of the tests. Rails implements `setup` and `teardown` as `ActiveSupport::Callbacks`. Which essentially means you need not only use `setup` and `teardown` as methods in your tests. You could specify them by using: +Let's break this test down so we can understand it. -* a block -* a method (like in the earlier example) -* a method name as a symbol -* a lambda +We start by calling the `:new` action on our Articles controller. This response should be successful, and we can verify the correct template is rendered including the form partial. -Let's see the earlier example by specifying `setup` callback by specifying a method name as a symbol: +After this we make a post request to the `:create` action of our Articles controller: ```ruby -require 'test_helper' - -class ArticlesControllerTest < ActionController::TestCase - - # called before every single test - setup :initialize_article - - # called after every single test - def teardown - @article = nil - end - - test "should show article" do - get :show, id: @article.id - assert_response :success - end - - test "should update article" do - patch :update, id: @article.id, article: {} - assert_redirected_to article_path(assigns(:article)) - end - - test "should destroy article" do - assert_difference('Article.count', -1) do - delete :destroy, id: @article.id - end - - assert_redirected_to articles_path - end - - private - - def initialize_article - @article = articles(:one) - end -end +post "/articles", article: {title: "can create", body: "article successfully."} +assert_response :redirect +follow_redirect! ``` -Testing Routes --------------- +The two lines following the request are to handle the redirect we setup when creating a new article. -Like everything else in your Rails application, it is recommended that you test your routes. Below are example tests for the routes of default `show` and `create` action of `Articles` controller above and it should look like: +NOTE: Don't forget to call `follow_redirect!` if you plan to make subsequent requests after a redirect is made. -```ruby -class ArticleRoutesTest < ActionController::TestCase - test "should route to article" do - assert_routing '/articles/1', { controller: "articles", action: "show", id: "1" } - end +Finally we can assert that our response was successful, template was rendered, and our new article is readable on the page. - test "should route to create article" do - assert_routing({ method: 'post', path: '/articles' }, { controller: "articles", action: "create" }) - end -end -``` +#### Taking it further + +We were able to successfully test a very small workflow for visiting our blog and creating a new article. If we wanted to take this further we could add tests for commenting, removing articles, or editting comments. Integration tests are a great place to experiment with all kinds of use-cases for our applications. Testing Your Mailers -------------------- @@ -1038,39 +1111,6 @@ class UserControllerTest < ActionController::TestCase end ``` -Testing helpers ---------------- - -In order to test helpers, all you need to do is check that the output of the -helper method matches what you'd expect. Tests related to the helpers are -located under the `test/helpers` directory. - -A helper test looks like so: - -```ruby -require 'test_helper' - -class UserHelperTest < ActionView::TestCase -end -``` - -A helper is just a simple module where you can define methods which are -available into your views. To test the output of the helper's methods, you just -have to use a mixin like this: - -```ruby -class UserHelperTest < ActionView::TestCase - include UserHelper - - test "should return the user name" do - # ... - end -end -``` - -Moreover, since the test class extends from `ActionView::TestCase`, you have -access to Rails' helper methods such as `link_to` or `pluralize`. - Testing Jobs ------------ @@ -1104,17 +1144,7 @@ no jobs have already been executed in the scope of each test. ### Custom Assertions And Testing Jobs Inside Other Components -Active Job ships with a bunch of custom assertions that can be used to lessen -the verbosity of tests: - -| Assertion | Purpose | -| -------------------------------------- | ------- | -| `assert_enqueued_jobs(number)` | Asserts that the number of enqueued jobs matches the given number. | -| `assert_performed_jobs(number)` | Asserts that the number of performed jobs matches the given number. | -| `assert_no_enqueued_jobs { ... }` | Asserts that no jobs have been enqueued. | -| `assert_no_performed_jobs { ... }` | Asserts that no jobs have been performed. | -| `assert_enqueued_with([args]) { ... }` | Asserts that the job passed in the block has been enqueued with the given arguments. | -| `assert_performed_with([args]) { ... }`| Asserts that the job passed in the block has been performed with the given arguments. | +Active Job ships with a bunch of custom assertions that can be used to lessen the verbosity of tests. For a full list of available assertions, see the API documentation for [`ActiveJob::TestHelper`](http://api.rubyonrails.org/classes/ActiveJob/TestHelper.html). It's a good practice to ensure that your jobs correctly get enqueued or performed wherever you invoke them (e.g. inside your controllers). This is precisely where diff --git a/railties/lib/rails/generators/named_base.rb b/railties/lib/rails/generators/named_base.rb index 397e1e73f1..36456e64f5 100644 --- a/railties/lib/rails/generators/named_base.rb +++ b/railties/lib/rails/generators/named_base.rb @@ -145,7 +145,7 @@ module Rails @route_url ||= class_path.collect {|dname| "/" + dname }.join + "/" + plural_file_name end - # Tries to retrieve the application name or simple return application. + # Tries to retrieve the application name or simply return application. def application_name if defined?(Rails) && Rails.application Rails.application.class.name.split('::').first.underscore diff --git a/railties/lib/rails/generators/rails/plugin/templates/Gemfile b/railties/lib/rails/generators/rails/plugin/templates/Gemfile index ab8b8925eb..84cef9525e 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/Gemfile +++ b/railties/lib/rails/generators/rails/plugin/templates/Gemfile @@ -45,3 +45,7 @@ end # gem 'byebug', group: [:development, :test] <%- end -%> <% end -%> + +<% if RUBY_PLATFORM.match(/bccwin|cygwin|emx|mingw|mswin|wince|java/) -%> +gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] +<% end -%> diff --git a/railties/test/generators/actions_test.rb b/railties/test/generators/actions_test.rb index 2206e389b5..c4b6441397 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -129,7 +129,7 @@ class ActionsTest < Rails::Generators::TestCase run_generator action :environment do - '# This wont be added' + _ = '# This wont be added'# assignment to silence parse-time warning "unused literal ignored" '# This will be added' end |