diff options
47 files changed, 269 insertions, 281 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index c27ef2dc5f..a9233014e4 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -312,7 +312,7 @@ * Stop converting empty arrays in `params` to `nil`. - This behaviour was introduced in response to CVE-2012-2660, CVE-2012-2694 + This behavior was introduced in response to CVE-2012-2660, CVE-2012-2694 and CVE-2013-0155 ActiveRecord now issues a safe query when passing an empty array into diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 47bcfdb1e9..bb3ad9b850 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -40,7 +40,7 @@ module ActionController # * <tt>:etag</tt>. # * <tt>:last_modified</tt>. # * <tt>:public</tt> By default the Cache-Control header is private, set this to - # +true+ if you want your application to be cachable by other devices (proxy caches). + # +true+ if you want your application to be cacheable by other devices (proxy caches). # * <tt>:template</tt> By default, the template digest for the current # controller/action is included in ETags. If the action renders a # different template, you can include its digest instead. If the action @@ -111,7 +111,7 @@ module ActionController # * <tt>:etag</tt>. # * <tt>:last_modified</tt>. # * <tt>:public</tt> By default the Cache-Control header is private, set this to - # +true+ if you want your application to be cachable by other devices (proxy caches). + # +true+ if you want your application to be cacheable by other devices (proxy caches). # * <tt>:template</tt> By default, the template digest for the current # controller/action is included in ETags. If the action renders a # different template, you can include its digest instead. If the action diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index 4038101fe0..b4da381d26 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -44,7 +44,7 @@ module ActionController # the output might look like this: # # 23 Aug 11:30 | Carolina Railhawks Soccer Match - # N/A | Carolina Railhaws Training Workshop + # N/A | Carolina Railhawks Training Workshop # module Helpers extend ActiveSupport::Concern diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index b7cd2b1a59..8a4ea70649 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -250,7 +250,7 @@ module ActionController private - # Returns the wrapper key which will be used to stored wrapped parameters. + # Returns the wrapper key which will be used to store wrapped parameters. def _wrapper_key _wrapper_options.name end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 356493bde6..a7bd3622ee 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -44,7 +44,7 @@ module ActionController #:nodoc: # during request. # # We may want to disable CSRF protection for APIs since they are typically - # designed to be state-less. That is, the requestion API client will handle + # designed to be state-less. That is, the request API client will handle # the session for you instead of Rails. # # The token parameter is named <tt>authenticity_token</tt> by default. The name and diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index a1f84e5ace..836bdead25 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -20,6 +20,8 @@ module ActionDispatch include ActionDispatch::Http::FilterParameters include ActionDispatch::Http::URL + HTTP_X_REQUEST_ID = "HTTP_X_REQUEST_ID".freeze # :nodoc: + autoload :Session, 'action_dispatch/request/session' autoload :Utils, 'action_dispatch/request/utils' @@ -50,7 +52,6 @@ module ActionDispatch @original_fullpath = nil @fullpath = nil @ip = nil - @request_id = nil end def check_path_parameters! @@ -180,7 +181,7 @@ module ActionDispatch # # request.headers["Content-Type"] # => "text/plain" def headers - Http::Headers.new(@env) + @headers ||= Http::Headers.new(@env) end # Returns a +String+ with the last requested path including their params. @@ -234,15 +235,19 @@ module ActionDispatch end alias :xhr? :xml_http_request? + # Returns the IP address of client as a +String+. def ip @ip ||= super end - # Originating IP address, usually set by the RemoteIp middleware. + # Returns the IP address of client as a +String+, + # usually set by the RemoteIp middleware. def remote_ip @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s end + ACTION_DISPATCH_REQUEST_ID = "action_dispatch.request_id".freeze # :nodoc: + # Returns the unique request id, which is based on either the X-Request-Id header that can # be generated by a firewall, load balancer, or web server or by the RequestId middleware # (which sets the action_dispatch.request_id environment variable). @@ -250,11 +255,19 @@ module ActionDispatch # This unique ID is useful for tracing a request from end-to-end as part of logging or debugging. # This relies on the rack variable set by the ActionDispatch::RequestId middleware. def request_id - @request_id ||= env["action_dispatch.request_id"] + env[ACTION_DISPATCH_REQUEST_ID] + end + + def request_id=(id) # :nodoc: + env[ACTION_DISPATCH_REQUEST_ID] = id end alias_method :uuid, :request_id + def x_request_id # :nodoc + @env[HTTP_X_REQUEST_ID] + end + # Returns the lowercase name of the HTTP server software. def server_software (@env['SERVER_SOFTWARE'] && /^([a-zA-Z]+)/ =~ @env['SERVER_SOFTWARE']) ? $1.downcase : nil diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index a895d1ab18..9e53a0f08b 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -274,7 +274,7 @@ module ActionDispatch # :nodoc: end # Turns the Response into a Rack-compatible array of the status, headers, - # and body. Allows explict splatting: + # and body. Allows explicit splatting: # # status, headers, body = *response def to_a diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index b9ca524309..1555ff72af 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -13,22 +13,23 @@ module ActionDispatch # from multiple pieces of the stack. class RequestId X_REQUEST_ID = "X-Request-Id".freeze # :nodoc: - ACTION_DISPATCH_REQUEST_ID = "action_dispatch.request_id".freeze # :nodoc: - HTTP_X_REQUEST_ID = "HTTP_X_REQUEST_ID".freeze # :nodoc: def initialize(app) @app = app end def call(env) - env[ACTION_DISPATCH_REQUEST_ID] = external_request_id(env) || internal_request_id - @app.call(env).tap { |_status, headers, _body| headers[X_REQUEST_ID] = env[ACTION_DISPATCH_REQUEST_ID] } + req = ActionDispatch::Request.new env + req.request_id = make_request_id(req.x_request_id) + @app.call(env).tap { |_status, headers, _body| headers[X_REQUEST_ID] = req.request_id } end private - def external_request_id(env) - if request_id = env[HTTP_X_REQUEST_ID].presence + def make_request_id(request_id) + if request_id.presence request_id.gsub(/[^\w\-]/, "".freeze).first(255) + else + internal_request_id end end diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index ecebd2d8c5..7cbc77f11f 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -682,7 +682,7 @@ class RenderTest < ActionController::TestCase def case_sensitive_file_system? fname = '.case_sensitive_file_system_test' FileUtils.touch(fname) - !File.exists?(fname.upcase) + !File.exist?(fname.upcase) ensure FileUtils.rm_f(fname) end diff --git a/activejob/test/cases/test_case_test.rb b/activejob/test/cases/test_case_test.rb index 0a3a20d5a0..ee816e1dd5 100644 --- a/activejob/test/cases/test_case_test.rb +++ b/activejob/test/cases/test_case_test.rb @@ -5,11 +5,11 @@ require 'jobs/nested_job' class ActiveJobTestCaseTest < ActiveJob::TestCase # this tests that this job class doesn't get its adapter set. - # that's the correct behaviour since we don't want to break - # the `class_attribute` inheritence - class TestClassAttributeInheritenceJob < ActiveJob::Base + # that's the correct behavior since we don't want to break + # the `class_attribute` inheritance + class TestClassAttributeInheritanceJob < ActiveJob::Base def self.queue_adapter=(*) - raise 'Attemping to break `class_attribute` inheritence, bad!' + raise 'Attemping to break `class_attribute` inheritance, bad!' end end diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 3c1510b31d..dddfd940bb 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -23,7 +23,7 @@ *Wojciech Wnętrzak* * Deprecate `ActiveModel::Errors#get`, `ActiveModel::Errors#set` and - `ActiveModel::Errors#[]=` methods that have inconsistent behaviour. + `ActiveModel::Errors#[]=` methods that have inconsistent behavior. *Wojciech Wnętrzak* diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 5b09101bca..e90490f8b6 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,13 @@ +* Pass `:extend` option for `has_and_belongs_to_many` associations to the underlying `has_many :through`. + + *Jaehyun Shin* + +* Deprecate `Relation#uniq` use `Relation#distinct` instead. + + See #9683. + + *Yves Senn* + * Allow single table inheritance instantiation to work when storing demodulized class names. diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index a3c30642d0..1ca648d48d 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -266,7 +266,6 @@ module ActiveRecord # others.find(*args) | X | X | X # others.exists? | X | X | X # others.distinct | X | X | X - # others.uniq | X | X | X # others.reset | X | X | X # # === Overriding generated methods @@ -1738,7 +1737,7 @@ module ActiveRecord hm_options[:through] = middle_reflection.name hm_options[:source] = join_model.right_reflection.name - [:before_add, :after_add, :before_remove, :after_remove, :autosave, :validate, :join_table, :class_name].each do |k| + [:before_add, :after_add, :before_remove, :after_remove, :autosave, :validate, :join_table, :class_name, :extend].each do |k| hm_options[k] = options[k] if options.key? k end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 9c5b7d937d..c918e88590 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -5,7 +5,6 @@ require 'active_support/dependencies' require 'active_support/descendants_tracker' require 'active_support/time' require 'active_support/core_ext/module/attribute_accessors' -require 'active_support/core_ext/class/delegating_attributes' require 'active_support/core_ext/array/extract_options' require 'active_support/core_ext/hash/deep_merge' require 'active_support/core_ext/hash/slice' diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 85648a7f8f..7d37313058 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -9,7 +9,7 @@ module ActiveRecord :extending, :unscope] SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :reordering, - :reverse_order, :distinct, :create_with, :uniq] + :reverse_order, :distinct, :create_with] CLAUSE_METHODS = [:where, :having, :from] INVALID_METHODS_FOR_DELETE_ALL = [:limit, :distinct, :offset, :group, :having] @@ -618,6 +618,7 @@ module ActiveRecord def uniq_value distinct_value end + deprecate uniq_value: :distinct_value # Compares two relations for equality. def ==(other) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 69ce5cdc2a..fd78db2e95 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -587,7 +587,7 @@ module ActiveRecord # # The two relations must be structurally compatible: they must be scoping the same model, and # they must differ only by +where+ (if no +group+ has been defined) or +having+ (if a +group+ is - # present). Neither relation may have a +limit+, +offset+, or +uniq+ set. + # present). Neither relation may have a +limit+, +offset+, or +distinct+ set. # # Post.where("id = 1").or(Post.where("id = 2")) # # SELECT `posts`.* FROM `posts` WHERE (('id = 1' OR 'id = 2')) @@ -790,6 +790,7 @@ module ActiveRecord spawn.distinct!(value) end alias uniq distinct + deprecate uniq: :distinct # Like #distinct, but modifies relation in place. def distinct!(value = true) # :nodoc: @@ -797,6 +798,7 @@ module ActiveRecord self end alias uniq! distinct! + deprecate uniq!: :distinct! # Used to extend a scope with additional methods, either through # a module or through a block provided. diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 0ecf2ddfd1..ffbf60e390 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1167,7 +1167,7 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_no_queries { assert client.accounts.empty? } end - def test_preloading_has_many_through_with_uniq + def test_preloading_has_many_through_with_distinct mary = Author.includes(:unique_categorized_posts).where(:id => authors(:mary).id).first assert_equal 1, mary.unique_categorized_posts.length assert_equal 1, mary.unique_categorized_post_ids.length diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index aea9207bfe..e584c94ad8 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -83,6 +83,16 @@ class DeveloperWithSymbolClassName < Developer has_and_belongs_to_many :projects, class_name: :ProjectWithSymbolsForKeys end +class DeveloperWithExtendOption < Developer + module NamedExtension + def category + 'sns' + end + end + + has_and_belongs_to_many :projects, extend: NamedExtension +end + class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :categories, :posts, :categories_posts, :developers, :projects, :developers_projects, :parrots, :pirates, :parrots_pirates, :treasures, :price_estimates, :tags, :taggings, :computers @@ -234,7 +244,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal developers, new_project.developers end - def test_habtm_unique_order_preserved + def test_habtm_distinct_order_preserved assert_equal developers(:poor_jamis, :jamis, :david), projects(:active_record).non_unique_developers assert_equal developers(:poor_jamis, :jamis, :david), projects(:active_record).developers end @@ -339,7 +349,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 'Yet Another Testing Title', another_post.title end - def test_uniq_after_the_fact + def test_distinct_after_the_fact dev = developers(:jamis) dev.projects << projects(:active_record) dev.projects << projects(:active_record) @@ -348,13 +358,13 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 1, dev.projects.distinct.size end - def test_uniq_before_the_fact + def test_distinct_before_the_fact projects(:active_record).developers << developers(:jamis) projects(:active_record).developers << developers(:david) assert_equal 3, projects(:active_record, :reload).developers.size end - def test_uniq_option_prevents_duplicate_push + def test_distinct_option_prevents_duplicate_push project = projects(:active_record) project.developers << developers(:jamis) project.developers << developers(:david) @@ -365,7 +375,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 3, project.developers.size end - def test_uniq_when_association_already_loaded + def test_distinct_when_association_already_loaded project = projects(:active_record) project.developers << [ developers(:jamis), developers(:david), developers(:jamis), developers(:david) ] assert_equal 3, Project.includes(:developers).find(project.id).developers.size @@ -577,6 +587,11 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal developers(:poor_jamis), projects(:active_record).developers.where("salary < 10000").first end + def test_association_with_extend_option + eponine = DeveloperWithExtendOption.create(name: 'Eponine') + assert_equal 'sns', eponine.projects.category + end + def test_replace_with_less david = developers(:david) david.projects = [projects(:action_controller)] diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 190cef55c4..1d545af5a5 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -960,7 +960,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal 1, category.categorizations.where(:special => true).count end - def test_joining_has_many_through_with_uniq + def test_joining_has_many_through_with_distinct mary = Author.joins(:unique_categorized_posts).where(:id => authors(:mary).id).first assert_equal 1, mary.unique_categorized_posts.length assert_equal 1, mary.unique_categorized_post_ids.length diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 213be50e67..5575419c35 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -35,12 +35,12 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert categories(:sti_test).authors.include?(authors(:mary)) end - def test_has_many_uniq_through_join_model + def test_has_many_distinct_through_join_model assert_equal 2, authors(:mary).categorized_posts.size assert_equal 1, authors(:mary).unique_categorized_posts.size end - def test_has_many_uniq_through_count + def test_has_many_distinct_through_count author = authors(:mary) assert !authors(:mary).unique_categorized_posts.loaded? assert_queries(1) { assert_equal 1, author.unique_categorized_posts.count } @@ -49,7 +49,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert !authors(:mary).unique_categorized_posts.loaded? end - def test_has_many_uniq_through_find + def test_has_many_distinct_through_find assert_equal 1, authors(:mary).unique_categorized_posts.to_a.size end @@ -625,7 +625,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert_equal [comments(:does_it_hurt)], authors(:david).special_post_comments end - def test_uniq_has_many_through_should_retain_order + def test_distinct_has_many_through_should_retain_order comment_ids = authors(:david).comments.map(&:id) assert_equal comment_ids.sort, authors(:david).ordered_uniq_comments.map(&:id) assert_equal comment_ids.sort.reverse, authors(:david).ordered_uniq_comments_desc.map(&:id) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 4306738670..31c31e4329 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1411,15 +1411,13 @@ class BasicsTest < ActiveRecord::TestCase end def test_uniq_delegates_to_scoped - scope = stub - Bird.stubs(:all).returns(mock(:uniq => scope)) - assert_equal scope, Bird.uniq + assert_deprecated do + assert_equal Bird.all.distinct, Bird.uniq + end end def test_distinct_delegates_to_scoped - scope = stub - Bird.stubs(:all).returns(mock(:distinct => scope)) - assert_equal scope, Bird.distinct + assert_equal Bird.all.distinct, Bird.distinct end def test_table_name_with_2_abstract_subclasses diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 8fc996ee74..b246eae5f5 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -359,7 +359,10 @@ class CalculationsTest < ActiveRecord::TestCase def test_count_with_distinct assert_equal 4, Account.select(:credit_limit).distinct.count - assert_equal 4, Account.select(:credit_limit).uniq.count + + assert_deprecated do + assert_equal 4, Account.select(:credit_limit).uniq.count + end end def test_count_with_aliased_attribute @@ -504,8 +507,8 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal [ topic.written_on ], relation.pluck(:written_on) end - def test_pluck_and_uniq - assert_equal [50, 53, 55, 60], Account.order(:credit_limit).uniq.pluck(:credit_limit) + def test_pluck_and_distinct + assert_equal [50, 53, 55, 60], Account.order(:credit_limit).distinct.pluck(:credit_limit) end def test_pluck_in_relation diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb index 45ead08bd5..ba4d9d2503 100644 --- a/activerecord/test/cases/relation/mutation_test.rb +++ b/activerecord/test/cases/relation/mutation_test.rb @@ -81,7 +81,7 @@ module ActiveRecord assert_equal [], relation.extending_values end - (Relation::SINGLE_VALUE_METHODS - [:lock, :reordering, :reverse_order, :create_with]).each do |method| + (Relation::SINGLE_VALUE_METHODS - [:lock, :reordering, :reverse_order, :create_with, :uniq]).each do |method| test "##{method}!" do assert relation.public_send("#{method}!", :foo).equal?(relation) assert_equal :foo, relation.public_send("#{method}_value") @@ -153,13 +153,22 @@ module ActiveRecord test 'distinct!' do relation.distinct! :foo assert_equal :foo, relation.distinct_value - assert_equal :foo, relation.uniq_value # deprecated access + + assert_deprecated do + assert_equal :foo, relation.uniq_value # deprecated access + end end test 'uniq! was replaced by distinct!' do - relation.uniq! :foo + assert_deprecated(/use distinct! instead/) do + relation.uniq! :foo + end + + assert_deprecated(/use distinct_value instead/) do + assert_equal :foo, relation.uniq_value # deprecated access + end + assert_equal :foo, relation.distinct_value - assert_equal :foo, relation.uniq_value # deprecated access end end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index b8e2041b6d..a4ca3ab637 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -908,7 +908,7 @@ class RelationTest < ActiveRecord::TestCase def test_delete_all_with_unpermitted_relation_raises_error assert_raises(ActiveRecord::ActiveRecordError) { Author.limit(10).delete_all } - assert_raises(ActiveRecord::ActiveRecordError) { Author.uniq.delete_all } + assert_raises(ActiveRecord::ActiveRecordError) { Author.distinct.delete_all } assert_raises(ActiveRecord::ActiveRecordError) { Author.group(:name).delete_all } assert_raises(ActiveRecord::ActiveRecordError) { Author.having('SUM(id) < 3').delete_all } assert_raises(ActiveRecord::ActiveRecordError) { Author.offset(10).delete_all } @@ -1493,14 +1493,17 @@ class RelationTest < ActiveRecord::TestCase assert_equal ['Foo', 'Foo'], query.map(&:name) assert_sql(/DISTINCT/) do assert_equal ['Foo'], query.distinct.map(&:name) - assert_equal ['Foo'], query.uniq.map(&:name) + assert_deprecated { assert_equal ['Foo'], query.uniq.map(&:name) } end assert_sql(/DISTINCT/) do assert_equal ['Foo'], query.distinct(true).map(&:name) - assert_equal ['Foo'], query.uniq(true).map(&:name) + assert_deprecated { assert_equal ['Foo'], query.uniq(true).map(&:name) } end assert_equal ['Foo', 'Foo'], query.distinct(true).distinct(false).map(&:name) - assert_equal ['Foo', 'Foo'], query.uniq(true).uniq(false).map(&:name) + + assert_deprecated do + assert_equal ['Foo', 'Foo'], query.uniq(true).uniq(false).map(&:name) + end end def test_doesnt_add_having_values_if_options_are_blank diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index d6fa651e39..2fac308d2e 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,25 @@ +* Add a bang version to `ActiveSupport::OrderedOptions` get methods which will raise + an `KeyError` if the value is `.blank?` + + Before: + + if (slack_url = Rails.application.secrets.slack_url).present?) + # Do something worthwhile + else + # Raise as important secret password is not specified + end + + After: + + slack_url = Rails.application.secrets.slack_url! + + *Aditya Sanghi*, *Gaurish Sharma* + +* Remove deprecated `Class#superclass_delegating_accessor`. + Use `Class#class_attribute` instead. + + *Akshay Vishnoi* + * Patch `Delegator` to work with `#try`. Fixes #5790. diff --git a/activesupport/lib/active_support/core_ext/class.rb b/activesupport/lib/active_support/core_ext/class.rb index c750a10bb2..ef903d59b5 100644 --- a/activesupport/lib/active_support/core_ext/class.rb +++ b/activesupport/lib/active_support/core_ext/class.rb @@ -1,3 +1,2 @@ require 'active_support/core_ext/class/attribute' -require 'active_support/core_ext/class/delegating_attributes' require 'active_support/core_ext/class/subclasses' diff --git a/activesupport/lib/active_support/core_ext/class/delegating_attributes.rb b/activesupport/lib/active_support/core_ext/class/delegating_attributes.rb deleted file mode 100644 index 1c305c5970..0000000000 --- a/activesupport/lib/active_support/core_ext/class/delegating_attributes.rb +++ /dev/null @@ -1,45 +0,0 @@ -require 'active_support/core_ext/kernel/singleton_class' -require 'active_support/core_ext/module/remove_method' -require 'active_support/core_ext/module/deprecation' - - -class Class - def superclass_delegating_accessor(name, options = {}) - # Create private _name and _name= methods that can still be used if the public - # methods are overridden. - _superclass_delegating_accessor("_#{name}", options) - - # Generate the public methods name, name=, and name?. - # These methods dispatch to the private _name, and _name= methods, making them - # overridable. - singleton_class.send(:define_method, name) { send("_#{name}") } - singleton_class.send(:define_method, "#{name}?") { !!send("_#{name}") } - singleton_class.send(:define_method, "#{name}=") { |value| send("_#{name}=", value) } - - # If an instance_reader is needed, generate public instance methods name and name?. - if options[:instance_reader] != false - define_method(name) { send("_#{name}") } - define_method("#{name}?") { !!send("#{name}") } - end - end - - deprecate superclass_delegating_accessor: :class_attribute - - private - # Take the object being set and store it in a method. This gives us automatic - # inheritance behavior, without having to store the object in an instance - # variable and look up the superclass chain manually. - def _stash_object_in_method(object, method, instance_reader = true) - singleton_class.remove_possible_method(method) - singleton_class.send(:define_method, method) { object } - remove_possible_method(method) - define_method(method) { object } if instance_reader - end - - def _superclass_delegating_accessor(name, options = {}) - singleton_class.send(:define_method, "#{name}=") do |value| - _stash_object_in_method(value, name, options[:instance_reader] != false) - end - send("#{name}=", nil) - end -end diff --git a/activesupport/lib/active_support/deprecation/behaviors.rb b/activesupport/lib/active_support/deprecation/behaviors.rb index 9f9dca8453..0cdc7e96f7 100644 --- a/activesupport/lib/active_support/deprecation/behaviors.rb +++ b/activesupport/lib/active_support/deprecation/behaviors.rb @@ -38,6 +38,18 @@ module ActiveSupport silence: ->(message, callstack) {}, } + # Behavior module allows to determine how to display deprecation messages. + # You can set any behaviors from +DEFAULT_BEHAVIORS+ constant or create + # custom behavior. Available behaviors: + # + # [+raise+] Raise <tt>ActiveSupport::DeprecationException</tt>. + # [+stderr+] Log all deprecation warnings to +$stderr+. + # [+log+] Log all deprecation warnings to +Rails.logger+. + # [+notify+] Use +ActiveSupport::Notifications+ to notify +deprecation.rails+. + # [+silence+] Do nothing. + # + # Setting behaviors only affects deprecations that happen after boot time. + # For more information you can read documentation for +behavior=+ method. module Behavior # Whether to print a backtrace along with the warning. attr_accessor :debug diff --git a/activesupport/lib/active_support/deprecation/proxy_wrappers.rb b/activesupport/lib/active_support/deprecation/proxy_wrappers.rb index dfdb8034e5..c6d2b5e795 100644 --- a/activesupport/lib/active_support/deprecation/proxy_wrappers.rb +++ b/activesupport/lib/active_support/deprecation/proxy_wrappers.rb @@ -104,7 +104,7 @@ module ActiveSupport end # DeprecatedConstantProxy transforms a constant into a deprecated one. It - # takes the names of an old (deprecated) constant and of a new contstant + # takes the names of an old (deprecated) constant and of a new constant # (both in string form) and optionally a deprecator. The deprecator defaults # to +ActiveSupport::Deprecator+ if none is specified. The deprecated constant # now returns the value of the new one. @@ -127,6 +127,11 @@ module ActiveSupport @deprecator = deprecator end + # Returns class of a new constant. + # + # PLANETS_POST_2006 = %w(mercury venus earth mars jupiter saturn uranus neptune) + # PLANETS = ActiveSupport::Deprecation::DeprecatedConstantProxy.new('PLANETS', 'PLANETS_POST_2006') + # PLANETS.class # => Array def class target.class end diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb index 4c0d1197fe..c63b61e97a 100644 --- a/activesupport/lib/active_support/duration.rb +++ b/activesupport/lib/active_support/duration.rb @@ -52,6 +52,10 @@ module ActiveSupport end end + # Returns the amount of seconds a duration covers as a string. + # For more information check to_i method. + # + # 1.day.to_s # => "86400" def to_s @value.to_s end diff --git a/activesupport/lib/active_support/multibyte/chars.rb b/activesupport/lib/active_support/multibyte/chars.rb index 3c0cf9f137..45cf6fc1ef 100644 --- a/activesupport/lib/active_support/multibyte/chars.rb +++ b/activesupport/lib/active_support/multibyte/chars.rb @@ -86,8 +86,14 @@ module ActiveSupport #:nodoc: @wrapped_string.split(*args).map { |i| self.class.new(i) } end - # Works like like <tt>String#slice!</tt>, but returns an instance of + # Works like <tt>String#slice!</tt>, but returns an instance of # Chars, or nil if the string was not modified. + # + # string = 'Welcome' + # string.mb_chars.slice!(3) # => #<ActiveSupport::Multibyte::Chars:0x000000038109b8 @wrapped_string="c"> + # string # => 'Welome' + # string.mb_chars.slice!(0..3) # => #<ActiveSupport::Multibyte::Chars:0x00000002eb80a0 @wrapped_string="Welo"> + # string # => 'me' def slice!(*args) chars(@wrapped_string.slice!(*args)) end diff --git a/activesupport/lib/active_support/ordered_options.rb b/activesupport/lib/active_support/ordered_options.rb index a33e2c58a9..ebf2a9bc29 100644 --- a/activesupport/lib/active_support/ordered_options.rb +++ b/activesupport/lib/active_support/ordered_options.rb @@ -31,7 +31,13 @@ module ActiveSupport if name_string.chomp!('=') self[name_string] = args.first else - self[name] + bangs = name_string.chomp!('!') + + if bangs + fetch(name_string.to_sym).presence || raise(KeyError.new("#{name_string} is blank.")) + else + self[name_string] + end end end diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb index 8b649c193f..411dd29df5 100644 --- a/activesupport/lib/active_support/testing/assertions.rb +++ b/activesupport/lib/active_support/testing/assertions.rb @@ -23,42 +23,42 @@ module ActiveSupport # result of what is evaluated in the yielded block. # # assert_difference 'Article.count' do - # post :create, article: {...} + # post :create, params: { article: {...} } # end # # An arbitrary expression is passed in and evaluated. # # assert_difference 'assigns(:article).comments(:reload).size' do - # post :create, comment: {...} + # post :create, params: { comment: {...} } # end # # An arbitrary positive or negative difference can be specified. # The default is <tt>1</tt>. # # assert_difference 'Article.count', -1 do - # post :delete, id: ... + # post :delete, params: { id: ... } # end # # An array of expressions can also be passed in and evaluated. # # assert_difference [ 'Article.count', 'Post.count' ], 2 do - # post :create, article: {...} + # post :create, params: { article: {...} } # end # # A lambda or a list of lambdas can be passed in and evaluated: # # assert_difference ->{ Article.count }, 2 do - # post :create, article: {...} + # post :create, params: { article: {...} } # end # # assert_difference [->{ Article.count }, ->{ Post.count }], 2 do - # post :create, article: {...} + # post :create, params: { article: {...} } # end # # An error message can be specified. # # assert_difference 'Article.count', -1, 'An Article should be destroyed' do - # post :delete, id: ... + # post :delete, params: { id: ... } # end def assert_difference(expression, difference = 1, message = nil, &block) expressions = Array(expression) @@ -81,13 +81,13 @@ module ActiveSupport # changed before and after invoking the passed in block. # # assert_no_difference 'Article.count' do - # post :create, article: invalid_attributes + # post :create, params: { article: invalid_attributes } # end # # An error message can be specified. # # assert_no_difference 'Article.count', 'An Article should not be created' do - # post :create, article: invalid_attributes + # post :create, params: { article: invalid_attributes } # end def assert_no_difference(expression, message = nil, &block) assert_difference expression, 0, message, &block diff --git a/activesupport/test/core_ext/class/delegating_attributes_test.rb b/activesupport/test/core_ext/class/delegating_attributes_test.rb deleted file mode 100644 index 447b1d10ad..0000000000 --- a/activesupport/test/core_ext/class/delegating_attributes_test.rb +++ /dev/null @@ -1,122 +0,0 @@ -require 'abstract_unit' -require 'active_support/core_ext/class/delegating_attributes' - -module DelegatingFixtures - class Parent - end - - class Child < Parent - ActiveSupport::Deprecation.silence do - superclass_delegating_accessor :some_attribute - end - end - - class Mokopuna < Child - end - - class PercysMom - ActiveSupport::Deprecation.silence do - superclass_delegating_accessor :superpower - end - end - - class Percy < PercysMom - end -end - -class DelegatingAttributesTest < ActiveSupport::TestCase - include DelegatingFixtures - attr_reader :single_class - - def setup - @single_class = Class.new(Object) - end - - def test_simple_accessor_declaration - assert_deprecated do - single_class.superclass_delegating_accessor :both - end - - # Class should have accessor and mutator - # the instance should have an accessor only - assert_respond_to single_class, :both - assert_respond_to single_class, :both= - assert single_class.public_instance_methods.map(&:to_s).include?("both") - assert !single_class.public_instance_methods.map(&:to_s).include?("both=") - end - - def test_simple_accessor_declaration_with_instance_reader_false - _instance_methods = single_class.public_instance_methods - - assert_deprecated do - single_class.superclass_delegating_accessor :no_instance_reader, :instance_reader => false - end - - assert_respond_to single_class, :no_instance_reader - assert_respond_to single_class, :no_instance_reader= - assert !_instance_methods.include?(:no_instance_reader) - assert !_instance_methods.include?(:no_instance_reader?) - assert !_instance_methods.include?(:_no_instance_reader) - end - - def test_working_with_simple_attributes - assert_deprecated do - single_class.superclass_delegating_accessor :both - end - - single_class.both = "HMMM" - - assert_equal "HMMM", single_class.both - assert_equal true, single_class.both? - - assert_equal "HMMM", single_class.new.both - assert_equal true, single_class.new.both? - - single_class.both = false - assert_equal false, single_class.both? - end - - def test_child_class_delegates_to_parent_but_can_be_overridden - parent = Class.new - - assert_deprecated do - parent.superclass_delegating_accessor :both - end - - child = Class.new(parent) - parent.both = "1" - assert_equal "1", child.both - - child.both = "2" - assert_equal "1", parent.both - assert_equal "2", child.both - - parent.both = "3" - assert_equal "3", parent.both - assert_equal "2", child.both - end - - def test_delegation_stops_at_the_right_level - assert_nil Percy.superpower - assert_nil PercysMom.superpower - - PercysMom.superpower = :heatvision - assert_equal :heatvision, Percy.superpower - end - - def test_delegation_stops_for_nil - Mokopuna.some_attribute = nil - Child.some_attribute="1" - - assert_equal "1", Child.some_attribute - assert_nil Mokopuna.some_attribute - ensure - Child.some_attribute=nil - end - - def test_deprecation_warning - assert_deprecated(/superclass_delegating_accessor is deprecated/) do - single_class.superclass_delegating_accessor :test_attribute - end - end -end diff --git a/activesupport/test/ordered_options_test.rb b/activesupport/test/ordered_options_test.rb index fdc745b23b..18767a3536 100644 --- a/activesupport/test/ordered_options_test.rb +++ b/activesupport/test/ordered_options_test.rb @@ -85,4 +85,19 @@ class OrderedOptionsTest < ActiveSupport::TestCase assert_equal 42, a.method(:blah=).call(42) assert_equal 42, a.method(:blah).call end + + def test_raises_with_bang + a = ActiveSupport::OrderedOptions.new + a[:foo] = :bar + assert a.respond_to?(:foo!) + + assert_nothing_raised { a.foo! } + assert_equal a.foo, a.foo! + + assert_raises(KeyError) do + a.foo = nil + a.foo! + end + assert_raises(KeyError) { a.non_existing_key! } + end end diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md index 92a4081233..bf3bf5d19e 100644 --- a/guides/source/action_mailer_basics.md +++ b/guides/source/action_mailer_basics.md @@ -344,7 +344,7 @@ The same format can be used to set carbon copy (Cc:) and blind carbon copy Sometimes you wish to show the name of the person instead of just their email address when they receive the email. The trick to doing that is to format the -email address in the format `"Full Name <email>"`. +email address in the format `"Full Name" <email>`. ```ruby def welcome_email(user) diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md index 8a026cecac..950bb5e358 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -325,9 +325,9 @@ You can also render a block of code within a partial layout instead of calling ` ```html+erb <% render(layout: 'box', locals: { article: @article }) do %> - <%= div_for(article) do %> + <div> <p><%= article.body %></p> - <% end %> + </div> <% end %> ``` diff --git a/guides/source/active_record_basics.md b/guides/source/active_record_basics.md index 6551ba0389..a227b54040 100644 --- a/guides/source/active_record_basics.md +++ b/guides/source/active_record_basics.md @@ -74,8 +74,8 @@ By default, Active Record uses some naming conventions to find out how the mapping between models and database tables should be created. Rails will pluralize your class names to find the respective database table. So, for a class `Book`, you should have a database table called **books**. The Rails -pluralization mechanisms are very powerful, being capable to pluralize (and -singularize) both regular and irregular words. When using class names composed +pluralization mechanisms are very powerful, being capable of pluralizing (and +singularizing) both regular and irregular words. When using class names composed of two or more words, the model class name should follow the Ruby conventions, using the CamelCase form, while the table name must contain the words separated by underscores. Examples: diff --git a/guides/source/association_basics.md b/guides/source/association_basics.md index 412cfd198a..1fe111f2a0 100644 --- a/guides/source/association_basics.md +++ b/guides/source/association_basics.md @@ -590,7 +590,7 @@ If you create an association some time after you build the underlying model, you If you create a `has_and_belongs_to_many` association, you need to explicitly create the joining table. Unless the name of the join table is explicitly specified by using the `:join_table` option, Active Record creates the name by using the lexical order of the class names. So a join between customer and order models will give the default join table name of "customers_orders" because "c" outranks "o" in lexical ordering. -WARNING: The precedence between model names is calculated using the `<` operator for `String`. This means that if the strings are of different lengths, and the strings are equal when compared up to the shortest length, then the longer string is considered of higher lexical precedence than the shorter one. For example, one would expect the tables "paper_boxes" and "papers" to generate a join table name of "papers_paper_boxes" because of the length of the name "paper_boxes", but it in fact generates a join table name of "paper_boxes_papers" (because the underscore '_' is lexicographically _less_ than 's' in common encodings). +WARNING: The precedence between model names is calculated using the `<=>` operator for `String`. This means that if the strings are of different lengths, and the strings are equal when compared up to the shortest length, then the longer string is considered of higher lexical precedence than the shorter one. For example, one would expect the tables "paper_boxes" and "papers" to generate a join table name of "papers_paper_boxes" because of the length of the name "paper_boxes", but it in fact generates a join table name of "paper_boxes_papers" (because the underscore '\_' is lexicographically _less_ than 's' in common encodings). Whatever the name, you must manually generate the join table with an appropriate migration. For example, consider these associations: @@ -620,7 +620,7 @@ class CreateAssembliesPartsJoinTable < ActiveRecord::Migration end ``` -We pass `id: false` to `create_table` because that table does not represent a model. That's required for the association to work properly. If you observe any strange behavior in a `has_and_belongs_to_many` association like mangled models IDs, or exceptions about conflicting IDs, chances are you forgot that bit. +We pass `id: false` to `create_table` because that table does not represent a model. That's required for the association to work properly. If you observe any strange behavior in a `has_and_belongs_to_many` association like mangled model IDs, or exceptions about conflicting IDs, chances are you forgot that bit. ### Controlling Association Scope @@ -793,7 +793,7 @@ If the associated object has already been retrieved from the database for this o ##### `association=(associate)` -The `association=` method assigns an associated object to this object. Behind the scenes, this means extracting the primary key from the associate object and setting this object's foreign key to the same value. +The `association=` method assigns an associated object to this object. Behind the scenes, this means extracting the primary key from the associated object and setting this object's foreign key to the same value. ```ruby @order.customer = @customer @@ -1138,7 +1138,7 @@ If the associated object has already been retrieved from the database for this o ##### `association=(associate)` -The `association=` method assigns an associated object to this object. Behind the scenes, this means extracting the primary key from this object and setting the associate object's foreign key to the same value. +The `association=` method assigns an associated object to this object. Behind the scenes, this means extracting the primary key from this object and setting the associated object's foreign key to the same value. ```ruby @supplier.account = @account @@ -1219,8 +1219,8 @@ Controls what happens to the associated object when its owner is destroyed: It's necessary not to set or leave `:nullify` option for those associations that have `NOT NULL` database constraints. If you don't set `dependent` to destroy such associations you won't be able to change the associated object -because initial associated object foreign key will be set to unallowed `NULL` -value. +because the initial associated object's foreign key will be set to the +unallowed `NULL` value. ##### `:foreign_key` @@ -1506,7 +1506,9 @@ The `collection.where` method finds objects within the collection based on the c ##### `collection.exists?(...)` -The `collection.exists?` method checks whether an object meeting the supplied conditions exists in the collection. It uses the same syntax and options as `ActiveRecord::Base.exists?`. +The `collection.exists?` method checks whether an object meeting the supplied +conditions exists in the collection. It uses the same syntax and options as +[`ActiveRecord::Base.exists?`](http://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-exists-3F). ##### `collection.build(attributes = {}, ...)` @@ -1617,9 +1619,10 @@ end By convention, Rails assumes that the column used to hold the primary key of the association is `id`. You can override this and explicitly specify the primary key with the `:primary_key` option. -Let's say that `users` table has `id` as the primary_key but it also has -`guid` column. And the requirement is that `todos` table should hold -`guid` column value and not `id` value. This can be achieved like this +Let's say the `users` table has `id` as the primary_key but it also +has a `guid` column. The requirement is that the `todos` table should +hold the `guid` column value as the foreign key and not `id` +value. This can be achieved like this: ```ruby class User < ActiveRecord::Base @@ -1627,8 +1630,8 @@ class User < ActiveRecord::Base end ``` -Now if we execute `@user.todos.create` then `@todo` record will have -`user_id` value as the `guid` value of `@user`. +Now if we execute `@todo = @user.todos.create` then the `@todo` +record's `user_id` value will be the `guid` value of `@user`. ##### `:source` @@ -2004,7 +2007,9 @@ The `collection.where` method finds objects within the collection based on the c ##### `collection.exists?(...)` -The `collection.exists?` method checks whether an object meeting the supplied conditions exists in the collection. It uses the same syntax and options as `ActiveRecord::Base.exists?`. +The `collection.exists?` method checks whether an object meeting the supplied +conditions exists in the collection. It uses the same syntax and options as +[`ActiveRecord::Base.exists?`](http://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-exists-3F). ##### `collection.build(attributes = {})` diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index 39a9826f21..3279c99c42 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -66,7 +66,7 @@ the core team will have to make a judgement call. That said, the distinction generally just affects which release your patch will get in to; we love feature submissions! They just won't get backported to maintenance branches. -If you'd like feedback on an idea for a feature before doing the work for make +If you'd like feedback on an idea for a feature before doing the work to make a patch, please send an email to the [rails-core mailing list](https://groups.google.com/forum/?fromgroups#!forum/rubyonrails-core). You might get no response, which means that everyone is indifferent. You might find @@ -79,17 +79,17 @@ discussions new features require. Helping to Resolve Existing Issues ---------------------------------- -As a next step beyond reporting issues, you can help the core team resolve existing issues. If you check the [Everyone's Issues](https://github.com/rails/rails/issues) list in GitHub Issues, you'll find lots of issues already requiring attention. What can you do for these? Quite a bit, actually: +As a next step beyond reporting issues, you can help the core team resolve existing issues. If you check the [issues list](https://github.com/rails/rails/issues) in GitHub Issues, you'll find lots of issues already requiring attention. What can you do for these? Quite a bit, actually: ### Verifying Bug Reports For starters, it helps just to verify bug reports. Can you reproduce the reported issue on your own computer? If so, you can add a comment to the issue saying that you're seeing the same thing. -If something is very vague, can you help squash it down into something specific? Maybe you can provide additional information to help reproduce a bug, or help by eliminating needless steps that aren't required to demonstrate the problem. +If an issue is very vague, can you help narrow it down to something more specific? Maybe you can provide additional information to help reproduce a bug, or help by eliminating needless steps that aren't required to demonstrate the problem. If you find a bug report without a test, it's very useful to contribute a failing test. This is also a great way to get started exploring the source code: looking at the existing test files will teach you how to write more tests. New tests are best contributed in the form of a patch, as explained later on in the "Contributing to the Rails Code" section. -Anything you can do to make bug reports more succinct or easier to reproduce is a help to folks trying to write code to fix those bugs - whether you end up writing the code yourself or not. +Anything you can do to make bug reports more succinct or easier to reproduce helps folks trying to write code to fix those bugs - whether you end up writing the code yourself or not. ### Testing Patches @@ -117,7 +117,7 @@ Once you're happy that the pull request contains a good change, comment on the G >I like the way you've restructured that code in generate_finder_sql - much nicer. The tests look good too. -If your comment simply says "+1", then odds are that other reviewers aren't going to take it too seriously. Show that you took the time to review the pull request. +If your comment simply reads "+1", then odds are that other reviewers aren't going to take it too seriously. Show that you took the time to review the pull request. Contributing to the Rails Documentation --------------------------------------- diff --git a/guides/source/debugging_rails_applications.md b/guides/source/debugging_rails_applications.md index a3c16c7660..96bf532868 100644 --- a/guides/source/debugging_rails_applications.md +++ b/guides/source/debugging_rails_applications.md @@ -778,7 +778,7 @@ will be stopped and you will have to start it again. ### Settings -`byebug` has a few available options to tweak its behaviour: +`byebug` has a few available options to tweak its behavior: * `set autoreload`: Reload source code when changed (defaults: true). * `set autolist`: Execute `list` command on every breakpoint (defaults: true). diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index 3680055f0c..17309d4b47 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -940,6 +940,8 @@ Please read [Pull Request #9978](https://github.com/rails/rails/pull/9978) for d * Rails 4.0 has removed the XML parameters parser. You will need to add the `actionpack-xml_parser` gem if you require this feature. +* Rails 4.0 changes the default `layout` lookup set using symbols or procs that return nil. To get the "no layout" behavior, return false instead of nil. + * Rails 4.0 changes the default memcached client from `memcache-client` to `dalli`. To upgrade, simply add `gem 'dalli'` to your `Gemfile`. * Rails 4.0 deprecates the `dom_id` and `dom_class` methods in controllers (they are fine in views). You will need to include the `ActionView::RecordIdentifier` module in controllers requiring this feature. diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 119a7cb829..061fafb156 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -174,6 +174,10 @@ module Rails options[value] ? '# ' : '' end + def keeps? + !options[:skip_keeps] + end + def sqlite3? !options[:skip_active_record] && options[:database] == 'sqlite3' end @@ -355,7 +359,7 @@ module Rails end def keep_file(destination) - create_file("#{destination}/.keep") unless options[:skip_keeps] + create_file("#{destination}/.keep") if keeps? end end end diff --git a/railties/lib/rails/generators/rails/app/templates/gitignore b/railties/lib/rails/generators/rails/app/templates/gitignore index 7c6f2098b8..e4a00ad181 100644 --- a/railties/lib/rails/generators/rails/app/templates/gitignore +++ b/railties/lib/rails/generators/rails/app/templates/gitignore @@ -15,5 +15,7 @@ <% end -%> # Ignore all logfiles and tempfiles. /log/* +<% if keeps? -%> !/log/.keep +<% end -%> /tmp diff --git a/railties/lib/rails/generators/rails/scaffold/USAGE b/railties/lib/rails/generators/rails/scaffold/USAGE index 1b2a944103..d2e495758d 100644 --- a/railties/lib/rails/generators/rails/scaffold/USAGE +++ b/railties/lib/rails/generators/rails/scaffold/USAGE @@ -36,6 +36,6 @@ Description: Examples: `rails generate scaffold post` - `rails generate scaffold post title body:text published:boolean` + `rails generate scaffold post title:string body:text published:boolean` `rails generate scaffold purchase amount:decimal tracking_id:integer:uniq` `rails generate scaffold user email:uniq password:digest` diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index 3bcf3894ce..3a2195035f 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -138,7 +138,11 @@ module SharedGeneratorTests def test_skip_keeps run_generator [destination_root, '--skip-keeps', '--full'] - assert_file('.gitignore') + + assert_file '.gitignore' do |content| + assert_no_match(/\.keep/, content) + end + assert_no_file('app/mailers/.keep') end end diff --git a/railties/test/generators_test.rb b/railties/test/generators_test.rb index ce75aba4eb..31a575749a 100644 --- a/railties/test/generators_test.rb +++ b/railties/test/generators_test.rb @@ -1,7 +1,7 @@ require 'generators/generators_test_helper' require 'rails/generators/rails/model/model_generator' require 'rails/generators/test_unit/model/model_generator' -require 'mocha/setup' # FIXME: stop using mocha +require 'minitest/mock' class GeneratorsTest < Rails::Generators::TestCase include GeneratorsTestHelper @@ -9,16 +9,20 @@ class GeneratorsTest < Rails::Generators::TestCase def setup @path = File.expand_path("lib", Rails.root) $LOAD_PATH.unshift(@path) + @mock_generator = MiniTest::Mock.new end def teardown $LOAD_PATH.delete(@path) + @mock_generator.verify end def test_simple_invoke assert File.exist?(File.join(@path, 'generators', 'model_generator.rb')) - TestUnit::Generators::ModelGenerator.expects(:start).with(["Account"], {}) - Rails::Generators.invoke("test_unit:model", ["Account"]) + @mock_generator.expect(:call, nil, [["Account"],{}]) + TestUnit::Generators::ModelGenerator.stub(:start, @mock_generator) do + Rails::Generators.invoke("test_unit:model", ["Account"]) + end end def test_invoke_when_generator_is_not_found @@ -47,19 +51,25 @@ class GeneratorsTest < Rails::Generators::TestCase def test_should_give_higher_preference_to_rails_generators assert File.exist?(File.join(@path, 'generators', 'model_generator.rb')) - Rails::Generators::ModelGenerator.expects(:start).with(["Account"], {}) - warnings = capture(:stderr){ Rails::Generators.invoke :model, ["Account"] } - assert warnings.empty? + @mock_generator.expect(:call, nil, [["Account"],{}]) + Rails::Generators::ModelGenerator.stub(:start, @mock_generator) do + warnings = capture(:stderr){ Rails::Generators.invoke :model, ["Account"] } + assert warnings.empty? + end end def test_invoke_with_default_values - Rails::Generators::ModelGenerator.expects(:start).with(["Account"], {}) - Rails::Generators.invoke :model, ["Account"] + @mock_generator.expect(:call, nil, [["Account"],{}]) + Rails::Generators::ModelGenerator.stub(:start, @mock_generator) do + Rails::Generators.invoke :model, ["Account"] + end end def test_invoke_with_config_values - Rails::Generators::ModelGenerator.expects(:start).with(["Account"], behavior: :skip) - Rails::Generators.invoke :model, ["Account"], behavior: :skip + @mock_generator.expect(:call, nil, [["Account"],{behavior: :skip}]) + Rails::Generators::ModelGenerator.stub(:start, @mock_generator) do + Rails::Generators.invoke :model, ["Account"], behavior: :skip + end end def test_find_by_namespace @@ -103,11 +113,13 @@ class GeneratorsTest < Rails::Generators::TestCase end def test_invoke_with_nested_namespaces - model_generator = mock('ModelGenerator') do - expects(:start).with(["Account"], {}) + model_generator = Minitest::Mock.new + model_generator.expect(:start, nil, [["Account"], {}]) + @mock_generator.expect(:call, model_generator, ['namespace', 'my:awesome']) + Rails::Generators.stub(:find_by_namespace, @mock_generator) do + Rails::Generators.invoke 'my:awesome:namespace', ["Account"] end - Rails::Generators.expects(:find_by_namespace).with('namespace', 'my:awesome').returns(model_generator) - Rails::Generators.invoke 'my:awesome:namespace', ["Account"] + model_generator.verify end def test_rails_generators_help_with_builtin_information @@ -173,8 +185,10 @@ class GeneratorsTest < Rails::Generators::TestCase def test_fallbacks_for_generators_on_invoke Rails::Generators.fallbacks[:shoulda] = :test_unit - TestUnit::Generators::ModelGenerator.expects(:start).with(["Account"], {}) - Rails::Generators.invoke "shoulda:model", ["Account"] + @mock_generator.expect(:call, nil, [["Account"],{}]) + TestUnit::Generators::ModelGenerator.stub(:start, @mock_generator) do + Rails::Generators.invoke "shoulda:model", ["Account"] + end ensure Rails::Generators.fallbacks.delete(:shoulda) end @@ -182,8 +196,10 @@ class GeneratorsTest < Rails::Generators::TestCase def test_nested_fallbacks_for_generators Rails::Generators.fallbacks[:shoulda] = :test_unit Rails::Generators.fallbacks[:super_shoulda] = :shoulda - TestUnit::Generators::ModelGenerator.expects(:start).with(["Account"], {}) - Rails::Generators.invoke "super_shoulda:model", ["Account"] + @mock_generator.expect(:call, nil, [["Account"],{}]) + TestUnit::Generators::ModelGenerator.stub(:start, @mock_generator) do + Rails::Generators.invoke "super_shoulda:model", ["Account"] + end ensure Rails::Generators.fallbacks.delete(:shoulda) Rails::Generators.fallbacks.delete(:super_shoulda) |