Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Don’t override already-defined default values when freezing #240

Merged
merged 3 commits into from

2 participants

@amarshall
  • Freezing a Virtus object no longer overwrites all existing default attribute values.
  • Add Attribute#defined? to cleanup AttributeSet#skip_default? a bit.
  • Make AttributeSet#set_default_attributes! behavior respect its documentation, which implied its only difference from set_default_attributes was that it additionally set lazy attributes, which was not entirely true since it also set non-lazy (and lazy) attributes that had already been defined.
amarshall added some commits
@amarshall amarshall Add Attribute#defined? to return if attribute value has been defined d84f4df
@amarshall amarshall Use Attribute#default? in AttributeSet#skip_default? 67ca515
@amarshall amarshall Don’t override already-defined default values when freezing
Further, make `AttributeSet#set_default_attributes!` behavior respect
its documentation, which implied its only difference from
`set_default_attributes` was that it additionally set lazy attributes,
which was not entirely true since it also set non-lazy (and lazy)
attributes that had already been defined.
c16ab82
@solnic solnic merged commit 45ef710 into solnic:master

1 check passed

Details default The Travis CI build passed
@solnic
Owner

@amarshall thank you Andrew for the patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 31, 2014
  1. @amarshall
  2. @amarshall
  3. @amarshall

    Don’t override already-defined default values when freezing

    amarshall authored
    Further, make `AttributeSet#set_default_attributes!` behavior respect
    its documentation, which implied its only difference from
    `set_default_attributes` was that it additionally set lazy attributes,
    which was not entirely true since it also set non-lazy (and lazy)
    attributes that had already been defined.
This page is out of date. Refresh to see the latest.
View
11 lib/virtus/attribute/accessor.rb
@@ -34,6 +34,17 @@ def self.extended(descendant)
descendant.instance_variable_set('@instance_variable_name', "@#{name}")
end
+ # Return if attribute value is defined
+ #
+ # @param [Object] instance
+ #
+ # @return [Boolean]
+ #
+ # @api public
+ def defined?(instance)
+ instance.instance_variable_defined?(instance_variable_name)
+ end
+
# Return value of the attribute
#
# @param [Object] instance
View
2  lib/virtus/attribute_set.rb
@@ -209,7 +209,7 @@ def finalize
# @api private
def skip_default?(object, attribute)
- attribute.lazy? || object.instance_variable_defined?(attribute.instance_variable_name)
+ attribute.lazy? || attribute.defined?(object)
end
# Merge the attributes into the index
View
2  lib/virtus/instance_methods.rb
@@ -191,7 +191,7 @@ def set_default_attributes
#
# @api public
def set_default_attributes!
- attribute_set.set_defaults(self, proc { |_| false })
+ attribute_set.set_defaults(self, proc { |object, attribute| attribute.defined?(object) })
self
end
View
20 spec/unit/virtus/attribute/defined_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe Virtus::Attribute, '#defined?' do
+ subject { object.defined?(instance) }
+
+ let(:object) { described_class.build(String, :name => name) }
+
+ let(:model) { Class.new { attr_accessor :test } }
+ let(:name) { :test }
+ let(:instance) { model.new }
+
+ context 'when the attribute value has not been defined' do
+ it { should be(false) }
+ end
+
+ context 'when the attribute value has been defined' do
+ before { instance.test = nil }
+ it { should be(true) }
+ end
+end
View
13 spec/unit/virtus/freeze_spec.rb
@@ -9,6 +9,7 @@
attribute :name, String, :default => 'foo', :lazy => true
attribute :age, Integer, :default => 30
+ attribute :rand, Float, :default => Proc.new { rand }
}
}
@@ -18,4 +19,16 @@
its(:name) { should eql('foo') }
its(:age) { should be(30) }
+
+ it "does not change dynamic default values" do
+ original_value = object.rand
+ object.freeze
+ expect(object.rand).to eq original_value
+ end
+
+ it "does not change default attributes that have been explicitly set" do
+ object.rand = 3.14
+ object.freeze
+ expect(object.rand).to eq 3.14
+ end
end
Something went wrong with that request. Please try again.