diff options
Diffstat (limited to 'activemodel')
| -rw-r--r-- | activemodel/CHANGELOG.md | 20 | ||||
| -rw-r--r-- | activemodel/Rakefile | 3 | ||||
| -rw-r--r-- | activemodel/activemodel.gemspec | 2 | ||||
| -rw-r--r-- | activemodel/examples/validations.rb | 1 | ||||
| -rw-r--r-- | activemodel/lib/active_model/dirty.rb | 40 | ||||
| -rw-r--r-- | activemodel/lib/active_model/errors.rb | 8 | ||||
| -rw-r--r-- | activemodel/lib/active_model/naming.rb | 12 | ||||
| -rw-r--r-- | activemodel/lib/active_model/secure_password.rb | 21 | ||||
| -rw-r--r-- | activemodel/lib/active_model/serializers/xml.rb | 2 | ||||
| -rw-r--r-- | activemodel/lib/active_model/validations/clusivity.rb | 16 | ||||
| -rw-r--r-- | activemodel/test/cases/dirty_test.rb | 20 | ||||
| -rw-r--r-- | activemodel/test/cases/railtie_test.rb | 2 | ||||
| -rw-r--r-- | activemodel/test/cases/secure_password_test.rb | 13 | ||||
| -rw-r--r-- | activemodel/test/cases/serializers/json_serialization_test.rb | 8 | ||||
| -rw-r--r-- | activemodel/test/cases/serializers/xml_serialization_test.rb | 3 | ||||
| -rw-r--r-- | activemodel/test/cases/validations/conditional_validation_test.rb | 14 | ||||
| -rw-r--r-- | activemodel/test/cases/validations/inclusion_validation_test.rb | 22 | ||||
| -rw-r--r-- | activemodel/test/models/topic.rb | 2 |
18 files changed, 151 insertions, 58 deletions
diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 0568e5d545..e8602ecbcf 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,23 @@ +* Fix `has_secure_password` to honor bcrypt-ruby's cost attribute. + + *T.J. Schuck* + +* Updated the `ActiveModel::Dirty#changed_attributes` method to be indifferent between using + symbols and strings as keys. + + *William Myers* + +* Added new API methods `reset_changes` and `changed_applied` to `ActiveModel::Dirty` + that control changes state. Previsously you needed to update internal + instance variables, but now API methods are available. + + *Bogdan Gusiev* + +* Fix has_secure_password. `password_confirmation` validations are triggered + even if no `password_confirmation` is set. + + *Vladimir Kiselev* + * `inclusion` / `exclusion` validations with ranges will only use the faster `Range#cover` for numerical ranges, and the more accurate `Range#include?` for non-numerical ones. diff --git a/activemodel/Rakefile b/activemodel/Rakefile index f72b949c64..407dda2ec3 100644 --- a/activemodel/Rakefile +++ b/activemodel/Rakefile @@ -13,9 +13,8 @@ end namespace :test do task :isolated do - ruby = File.join(*RbConfig::CONFIG.values_at('bindir', 'RUBY_INSTALL_NAME')) Dir.glob("#{dir}/test/**/*_test.rb").all? do |file| - sh(ruby, '-w', "-I#{dir}/lib", "-I#{dir}/test", file) + sh(Gem.ruby, '-w', "-I#{dir}/lib", "-I#{dir}/test", file) end or raise "Failures" end end diff --git a/activemodel/activemodel.gemspec b/activemodel/activemodel.gemspec index 51655fe3da..11e755649c 100644 --- a/activemodel/activemodel.gemspec +++ b/activemodel/activemodel.gemspec @@ -5,7 +5,7 @@ Gem::Specification.new do |s| s.name = 'activemodel' s.version = version s.summary = 'A toolkit for building modeling frameworks (part of Rails).' - s.description = 'A toolkit for building modeling frameworks like Active Record. Rich support for attributes, callbacks, validations, observers, serialization, internationalization, and testing.' + s.description = 'A toolkit for building modeling frameworks like Active Record. Rich support for attributes, callbacks, validations, serialization, internationalization, and testing.' s.required_ruby_version = '>= 1.9.3' diff --git a/activemodel/examples/validations.rb b/activemodel/examples/validations.rb index c94cd17e18..b8e74acd5e 100644 --- a/activemodel/examples/validations.rb +++ b/activemodel/examples/validations.rb @@ -1,3 +1,4 @@ +require File.expand_path('../../../load_paths', __FILE__) require 'active_model' class Person diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index ea5ddf71de..c5f1b3f11a 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -14,13 +14,9 @@ module ActiveModel # track. # * Call <tt>attr_name_will_change!</tt> before each change to the tracked # attribute. - # - # If you wish to also track previous changes on save or update, you need to - # add: - # - # @previously_changed = changes - # - # inside of your save or update method. + # * Call <tt>changes_applied</tt> after the changes are persisted. + # * Call <tt>reset_changes</tt> when you want to reset the changes + # information. # # A minimal implementation could be: # @@ -39,8 +35,12 @@ module ActiveModel # end # # def save - # @previously_changed = changes - # @changed_attributes.clear + # # do persistence work + # changes_applied + # end + # + # def reload! + # reset_changes # end # end # @@ -65,6 +65,12 @@ module ActiveModel # person.changed? # => false # person.name_changed? # => false # + # Reset the changes: + # + # person.previous_changes # => {"name" => ["Uncle Bob", "Bill"]} + # person.reload! + # person.previous_changes # => {} + # # Assigning the same value leaves the attribute unchanged: # # person.name = 'Bill' @@ -129,7 +135,7 @@ module ActiveModel # person.save # person.previous_changes # => {"name" => ["bob", "robert"]} def previous_changes - @previously_changed + @previously_changed ||= {} end # Returns a hash of the attributes with unsaved changes indicating their original @@ -139,7 +145,7 @@ module ActiveModel # person.name = 'robert' # person.changed_attributes # => {"name" => "bob"} def changed_attributes - @changed_attributes ||= {} + @changed_attributes ||= ActiveSupport::HashWithIndifferentAccess.new end # Handle <tt>*_changed?</tt> for +method_missing+. @@ -154,6 +160,18 @@ module ActiveModel private + # Removes current changes and makes them accessible through +previous_changes+. + def changes_applied + @previously_changed = changes + @changed_attributes = {} + end + + # Removes all dirty data: current changes and previous changes + def reset_changes + @previously_changed = {} + @changed_attributes = {} + end + # Handle <tt>*_change</tt> for +method_missing+. def attribute_change(attr) [changed_attributes[attr], __send__(attr)] if attribute_changed?(attr) diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index e608a649f8..cf7551e4f4 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -238,8 +238,8 @@ module ActiveModel # object. You can pass the <tt>:full_messages</tt> option. This determines # if the json object should contain full messages or not (false by default). # - # person.as_json # => {:name=>["can not be nil"]} - # person.as_json(full_messages: true) # => {:name=>["name can not be nil"]} + # person.errors.as_json # => {:name=>["can not be nil"]} + # person.errors.as_json(full_messages: true) # => {:name=>["name can not be nil"]} def as_json(options=nil) to_hash(options && options[:full_messages]) end @@ -247,8 +247,8 @@ module ActiveModel # Returns a Hash of attributes with their error messages. If +full_messages+ # is +true+, it will contain full messages (see +full_message+). # - # person.to_hash # => {:name=>["can not be nil"]} - # person.to_hash(true) # => {:name=>["name can not be nil"]} + # person.errors.to_hash # => {:name=>["can not be nil"]} + # person.errors.to_hash(true) # => {:name=>["name can not be nil"]} def to_hash(full_messages = false) if full_messages messages = {} diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index bc9edf4a56..198efc5088 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -262,10 +262,10 @@ module ActiveModel # namespaced models regarding whether it's inside isolated engine. # # # For isolated engine: - # ActiveModel::Naming.singular_route_key(Blog::Post) #=> post + # ActiveModel::Naming.singular_route_key(Blog::Post) #=> "post" # # # For shared engine: - # ActiveModel::Naming.singular_route_key(Blog::Post) #=> blog_post + # ActiveModel::Naming.singular_route_key(Blog::Post) #=> "blog_post" def self.singular_route_key(record_or_class) model_name_from_record_or_class(record_or_class).singular_route_key end @@ -274,10 +274,10 @@ module ActiveModel # namespaced models regarding whether it's inside isolated engine. # # # For isolated engine: - # ActiveModel::Naming.route_key(Blog::Post) #=> posts + # ActiveModel::Naming.route_key(Blog::Post) #=> "posts" # # # For shared engine: - # ActiveModel::Naming.route_key(Blog::Post) #=> blog_posts + # ActiveModel::Naming.route_key(Blog::Post) #=> "blog_posts" # # The route key also considers if the noun is uncountable and, in # such cases, automatically appends _index. @@ -289,10 +289,10 @@ module ActiveModel # namespaced models regarding whether it's inside isolated engine. # # # For isolated engine: - # ActiveModel::Naming.param_key(Blog::Post) #=> post + # ActiveModel::Naming.param_key(Blog::Post) #=> "post" # # # For shared engine: - # ActiveModel::Naming.param_key(Blog::Post) #=> blog_post + # ActiveModel::Naming.param_key(Blog::Post) #=> "blog_post" def self.param_key(record_or_class) model_name_from_record_or_class(record_or_class).param_key end diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 7156f1bb30..7e694b5c50 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -2,7 +2,9 @@ module ActiveModel module SecurePassword extend ActiveSupport::Concern - class << self; attr_accessor :min_cost; end + class << self + attr_accessor :min_cost # :nodoc: + end self.min_cost = false module ClassMethods @@ -18,9 +20,9 @@ module ActiveModel # value to the password_confirmation attribute and the validation # will not be triggered. # - # You need to add bcrypt-ruby (~> 3.0.0) to Gemfile to use #has_secure_password: + # You need to add bcrypt-ruby (~> 3.1.2) to Gemfile to use #has_secure_password: # - # gem 'bcrypt-ruby', '~> 3.0.0' + # gem 'bcrypt-ruby', '~> 3.1.2' # # Example using Active Record (which automatically includes ActiveModel::SecurePassword): # @@ -44,7 +46,6 @@ module ActiveModel # This is to avoid ActiveModel (and by extension the entire framework) # being dependent on a binary library. begin - gem 'bcrypt-ruby', '~> 3.0.0' require 'bcrypt' rescue LoadError $stderr.puts "You don't have bcrypt-ruby installed in your application. Please add it to your Gemfile and run bundle install" @@ -56,9 +57,9 @@ module ActiveModel include InstanceMethodsOnActivation if options.fetch(:validations, true) - validates_confirmation_of :password, if: lambda { |m| m.password.present? } + validates_confirmation_of :password, if: :should_confirm_password? validates_presence_of :password, on: :create - validates_presence_of :password_confirmation, if: lambda { |m| m.password.present? } + validates_presence_of :password_confirmation, if: :should_confirm_password? before_create { raise "Password digest missing on new record" if password_digest.blank? } end @@ -101,7 +102,7 @@ module ActiveModel def password=(unencrypted_password) unless unencrypted_password.blank? @password = unencrypted_password - cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine::DEFAULT_COST + cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine.cost self.password_digest = BCrypt::Password.create(unencrypted_password, cost: cost) end end @@ -109,6 +110,12 @@ module ActiveModel def password_confirmation=(unencrypted_password) @password_confirmation = unencrypted_password end + + private + + def should_confirm_password? + password_confirmation && password.present? + end end end end diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb index 2803f69b6f..2864c2ba11 100644 --- a/activemodel/lib/active_model/serializers/xml.rb +++ b/activemodel/lib/active_model/serializers/xml.rb @@ -205,7 +205,7 @@ module ActiveModel Serializer.new(self, options).serialize(&block) end - # Sets the model +attributes+ from a JSON string. Returns +self+. + # Sets the model +attributes+ from an XML string. Returns +self+. # # class Person # include ActiveModel::Serializers::Xml diff --git a/activemodel/lib/active_model/validations/clusivity.rb b/activemodel/lib/active_model/validations/clusivity.rb index 1c35cb7c35..fd6cc1edb4 100644 --- a/activemodel/lib/active_model/validations/clusivity.rb +++ b/activemodel/lib/active_model/validations/clusivity.rb @@ -30,12 +30,18 @@ module ActiveModel @delimiter ||= options[:in] || options[:within] end - # In Ruby 1.9 <tt>Range#include?</tt> on non-numeric ranges checks all possible values in the - # range for equality, which is slower but more accurate. <tt>Range#cover?</tt> uses - # the previous logic of comparing a value with the range endpoints, which is fast - # but is only accurate on numeric ranges. + # In Ruby 1.9 <tt>Range#include?</tt> on non-number-or-time-ish ranges checks all + # possible values in the range for equality, which is slower but more accurate. + # <tt>Range#cover?</tt> uses the previous logic of comparing a value with the range + # endpoints, which is fast but is only accurate on Numeric, Time, or DateTime ranges. def inclusion_method(enumerable) - (enumerable.is_a?(Range) && enumerable.first.is_a?(Numeric)) ? :cover? : :include? + return :include? unless enumerable.is_a?(Range) + case enumerable.first + when Numeric, Time, DateTime + :cover? + else + :include? + end end end end diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb index ba45089cca..a90d0b1299 100644 --- a/activemodel/test/cases/dirty_test.rb +++ b/activemodel/test/cases/dirty_test.rb @@ -3,11 +3,12 @@ require "cases/helper" class DirtyTest < ActiveModel::TestCase class DirtyModel include ActiveModel::Dirty - define_attribute_methods :name, :color + define_attribute_methods :name, :color, :size def initialize @name = nil @color = nil + @size = nil end def name @@ -28,9 +29,17 @@ class DirtyTest < ActiveModel::TestCase @color = val end + def size + @size + end + + def size=(val) + attribute_will_change!(:size) unless val == @size + @size = val + end + def save - @previously_changed = changes - @changed_attributes.clear + changes_applied end end @@ -125,4 +134,9 @@ class DirtyTest < ActiveModel::TestCase assert_equal ["Otto", "Mr. Manfredgensonton"], @model.name_change assert_equal @model.name_was, "Otto" end + + test "using attribute_will_change! with a symbol" do + @model.size = 1 + assert @model.size_changed? + end end diff --git a/activemodel/test/cases/railtie_test.rb b/activemodel/test/cases/railtie_test.rb index 0643fa775d..96b3b07e50 100644 --- a/activemodel/test/cases/railtie_test.rb +++ b/activemodel/test/cases/railtie_test.rb @@ -8,7 +8,7 @@ class RailtieTest < ActiveModel::TestCase require 'active_model/railtie' # Set a fake logger to avoid creating the log directory automatically - fake_logger = mock() + fake_logger = Logger.new(nil) @app ||= Class.new(::Rails::Application) do config.eager_load = false diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index 0b900d934d..41d0b2263e 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -82,6 +82,14 @@ class SecurePasswordTest < ActiveModel::TestCase assert_equal BCrypt::Engine::DEFAULT_COST, @user.password_digest.cost end + test "Password digest cost honors bcrypt cost attribute when min_cost is false" do + ActiveModel::SecurePassword.min_cost = false + BCrypt::Engine.cost = 5 + + @user.password = "secret" + assert_equal BCrypt::Engine.cost, @user.password_digest.cost + end + test "Password digest cost can be set to bcrypt min cost to speed up tests" do ActiveModel::SecurePassword.min_cost = true @@ -95,6 +103,11 @@ class SecurePasswordTest < ActiveModel::TestCase assert @user.valid?(:update), "user should be valid" end + test "password_confirmation validations will not be triggered if password_confirmation is not sent" do + @user.password = "password" + assert @user.valid?(:create) + end + test "will not save if confirmation is blank but password is not" do @user.password = "password" @user.password_confirmation = "" diff --git a/activemodel/test/cases/serializers/json_serialization_test.rb b/activemodel/test/cases/serializers/json_serialization_test.rb index f0347081ee..bc185c737f 100644 --- a/activemodel/test/cases/serializers/json_serialization_test.rb +++ b/activemodel/test/cases/serializers/json_serialization_test.rb @@ -155,12 +155,6 @@ class JsonSerializationTest < ActiveModel::TestCase end end - test "as_json should keep the default order in the hash" do - json = @contact.as_json - - assert_equal %w(name age created_at awesome preferences), json.keys - end - test "from_json should work without a root (class attribute)" do json = @contact.to_json result = Contact.new.from_json(json) @@ -204,7 +198,7 @@ class JsonSerializationTest < ActiveModel::TestCase assert_no_match %r{"preferences":}, json end - test "custom as_json options should be extendible" do + test "custom as_json options should be extensible" do def @contact.as_json(options = {}); super(options.merge(only: [:name])); end json = @contact.to_json diff --git a/activemodel/test/cases/serializers/xml_serialization_test.rb b/activemodel/test/cases/serializers/xml_serialization_test.rb index c4cfb0c255..11ee17bb27 100644 --- a/activemodel/test/cases/serializers/xml_serialization_test.rb +++ b/activemodel/test/cases/serializers/xml_serialization_test.rb @@ -53,8 +53,7 @@ class XmlSerializationTest < ActiveModel::TestCase @contact.address.city = "Springfield" @contact.address.apt_number = 35 @contact.friends = [Contact.new, Contact.new] - @related_contact = SerializableContact.new - @contact.contact = @related_contact + @contact.contact = SerializableContact.new end test "should serialize default root" do diff --git a/activemodel/test/cases/validations/conditional_validation_test.rb b/activemodel/test/cases/validations/conditional_validation_test.rb index 41a4c33727..5049d6dd61 100644 --- a/activemodel/test/cases/validations/conditional_validation_test.rb +++ b/activemodel/test/cases/validations/conditional_validation_test.rb @@ -23,7 +23,7 @@ class ConditionalValidationTest < ActiveModel::TestCase Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", unless: :condition_is_true) t = Topic.new("title" => "uhohuhoh", "content" => "whatever") assert t.valid? - assert t.errors[:title].empty? + assert_empty t.errors[:title] end def test_if_validation_using_method_false @@ -31,7 +31,7 @@ class ConditionalValidationTest < ActiveModel::TestCase Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", if: :condition_is_true_but_its_not) t = Topic.new("title" => "uhohuhoh", "content" => "whatever") assert t.valid? - assert t.errors[:title].empty? + assert_empty t.errors[:title] end def test_unless_validation_using_method_false @@ -57,7 +57,7 @@ class ConditionalValidationTest < ActiveModel::TestCase Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", unless: "a = 1; a == 1") t = Topic.new("title" => "uhohuhoh", "content" => "whatever") assert t.valid? - assert t.errors[:title].empty? + assert_empty t.errors[:title] end def test_if_validation_using_string_false @@ -65,7 +65,7 @@ class ConditionalValidationTest < ActiveModel::TestCase Topic.validates_length_of(:title, maximum: 5, too_long: "hoo %{count}", if: "false") t = Topic.new("title" => "uhohuhoh", "content" => "whatever") assert t.valid? - assert t.errors[:title].empty? + assert_empty t.errors[:title] end def test_unless_validation_using_string_false @@ -93,7 +93,7 @@ class ConditionalValidationTest < ActiveModel::TestCase unless: Proc.new { |r| r.content.size > 4 }) t = Topic.new("title" => "uhohuhoh", "content" => "whatever") assert t.valid? - assert t.errors[:title].empty? + assert_empty t.errors[:title] end def test_if_validation_using_block_false @@ -102,7 +102,7 @@ class ConditionalValidationTest < ActiveModel::TestCase if: Proc.new { |r| r.title != "uhohuhoh"}) t = Topic.new("title" => "uhohuhoh", "content" => "whatever") assert t.valid? - assert t.errors[:title].empty? + assert_empty t.errors[:title] end def test_unless_validation_using_block_false @@ -124,7 +124,7 @@ class ConditionalValidationTest < ActiveModel::TestCase t = Topic.new assert t.invalid?, "A topic without a title should not be valid" - assert t.errors[:author_name].empty?, "A topic without an 'important' title should not require an author" + assert_empty t.errors[:author_name], "A topic without an 'important' title should not require an author" t.title = "Just a title" assert t.valid?, "A topic with a basic title should be valid" diff --git a/activemodel/test/cases/validations/inclusion_validation_test.rb b/activemodel/test/cases/validations/inclusion_validation_test.rb index 01a373d85d..8b90856869 100644 --- a/activemodel/test/cases/validations/inclusion_validation_test.rb +++ b/activemodel/test/cases/validations/inclusion_validation_test.rb @@ -1,5 +1,6 @@ # encoding: utf-8 require 'cases/helper' +require 'active_support/all' require 'models/topic' require 'models/person' @@ -20,6 +21,27 @@ class InclusionValidationTest < ActiveModel::TestCase assert Topic.new("title" => "bbb", "content" => "abc").valid? end + def test_validates_inclusion_of_time_range + Topic.validates_inclusion_of(:created_at, in: 1.year.ago..Time.now) + assert Topic.new(title: 'aaa', created_at: 2.years.ago).invalid? + assert Topic.new(title: 'aaa', created_at: 3.months.ago).valid? + assert Topic.new(title: 'aaa', created_at: 37.weeks.from_now).invalid? + end + + def test_validates_inclusion_of_date_range + Topic.validates_inclusion_of(:created_at, in: 1.year.until(Date.today)..Date.today) + assert Topic.new(title: 'aaa', created_at: 2.years.until(Date.today)).invalid? + assert Topic.new(title: 'aaa', created_at: 3.months.until(Date.today)).valid? + assert Topic.new(title: 'aaa', created_at: 37.weeks.since(Date.today)).invalid? + end + + def test_validates_inclusion_of_date_time_range + Topic.validates_inclusion_of(:created_at, in: 1.year.until(DateTime.current)..DateTime.current) + assert Topic.new(title: 'aaa', created_at: 2.years.until(DateTime.current)).invalid? + assert Topic.new(title: 'aaa', created_at: 3.months.until(DateTime.current)).valid? + assert Topic.new(title: 'aaa', created_at: 37.weeks.since(DateTime.current)).invalid? + end + def test_validates_inclusion_of Topic.validates_inclusion_of(:title, in: %w( a b c d e f g )) diff --git a/activemodel/test/models/topic.rb b/activemodel/test/models/topic.rb index c9af78f595..1411a093e9 100644 --- a/activemodel/test/models/topic.rb +++ b/activemodel/test/models/topic.rb @@ -6,7 +6,7 @@ class Topic super | [ :message ] end - attr_accessor :title, :author_name, :content, :approved + attr_accessor :title, :author_name, :content, :approved, :created_at attr_accessor :after_validation_performed after_validation :perform_after_validation |