diff --git a/VERSION b/VERSION index 054dfe3..95a5509 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.1.0.beta2 +1.1.0.beta3 diff --git a/benchmarks/dirty.rb b/benchmarks/dirty.rb index d5ffe2e..644f84d 100644 --- a/benchmarks/dirty.rb +++ b/benchmarks/dirty.rb @@ -111,8 +111,8 @@ begin run_benchmark end set_dirty(false) + puts "\nwith use_dirty false" end - puts "\nwith use_dirty false" run_benchmark end diff --git a/history.txt b/history.txt index 15059c6..49aa82b 100644 --- a/history.txt +++ b/history.txt @@ -1,6 +1,7 @@ == 1.1.0.beta3 * Major changes: + * Fast Dirty Tracking! Many thanks to @sobakasu (Andrew Williams) * Default CouchRest Model type field now set to 'model' instead of 'couchrest-type'. * Minor enhancements: diff --git a/lib/couchrest/model/base.rb b/lib/couchrest/model/base.rb index 3e585cc..b3b1b17 100644 --- a/lib/couchrest/model/base.rb +++ b/lib/couchrest/model/base.rb @@ -40,10 +40,6 @@ module CouchRest subclasses << subklass end - # Accessors - attr_accessor :casted_by - - # Instantiate a new CouchRest::Model::Base by preparing all properties # using the provided document hash. # diff --git a/lib/couchrest/model/casted_model.rb b/lib/couchrest/model/casted_model.rb index 4679954..990f454 100644 --- a/lib/couchrest/model/casted_model.rb +++ b/lib/couchrest/model/casted_model.rb @@ -11,31 +11,31 @@ module CouchRest::Model include CouchRest::Model::Associations include CouchRest::Model::Validations include CouchRest::Model::Dirty - attr_accessor :casted_by + # attr_accessor :casted_by end - + def initialize(keys = {}) raise StandardError unless self.is_a? Hash prepare_all_attributes(keys) super() end - + def []= key, value - couchrest_attribute_will_change!(key) if use_dirty && self[key] != value + couchrest_attribute_will_change!(key) if self[key] != value super(key.to_s, value) end - + def [] key super(key.to_s) end - + # Gets a reference to the top level extended # document that a model is saved inside of def base_doc return nil unless @casted_by @casted_by.base_doc end - + # False if the casted model has already # been saved in the containing document def new? diff --git a/lib/couchrest/model/configuration.rb b/lib/couchrest/model/configuration.rb index e68c53d..027bf3c 100644 --- a/lib/couchrest/model/configuration.rb +++ b/lib/couchrest/model/configuration.rb @@ -11,13 +11,11 @@ module CouchRest add_config :model_type_key add_config :mass_assign_any_attribute add_config :auto_update_design_doc - add_config :use_dirty configure do |config| config.model_type_key = 'model' # was 'couchrest-type' config.mass_assign_any_attribute = false config.auto_update_design_doc = true - config.use_dirty = true end end diff --git a/lib/couchrest/model/dirty.rb b/lib/couchrest/model/dirty.rb index e0a13b0..ef74e72 100644 --- a/lib/couchrest/model/dirty.rb +++ b/lib/couchrest/model/dirty.rb @@ -22,7 +22,7 @@ module CouchRest def use_dirty? bdoc = base_doc - bdoc && bdoc.use_dirty && !bdoc.disable_dirty + bdoc && !bdoc.disable_dirty end def couchrest_attribute_will_change!(attr) @@ -30,13 +30,13 @@ module CouchRest attribute_will_change!(attr) couchrest_parent_will_change! end - + def couchrest_parent_will_change! @casted_by.couchrest_attribute_will_change!(casted_by_attribute) if @casted_by end private - + # return the attribute name this object is referenced by in the parent def casted_by_attribute return @casted_by_attribute if @casted_by_attribute diff --git a/lib/couchrest/model/persistence.rb b/lib/couchrest/model/persistence.rb index 7679f8d..6374722 100644 --- a/lib/couchrest/model/persistence.rb +++ b/lib/couchrest/model/persistence.rb @@ -30,7 +30,7 @@ module CouchRest def update(options = {}) raise "Calling #{self.class.name}#update on document that has not been created!" if self.new? return false unless perform_validations(options) - return true if use_dirty? && !self.changed? + return true if !self.changed? _run_update_callbacks do _run_save_callbacks do result = database.save_doc(self) diff --git a/lib/couchrest/model/properties.rb b/lib/couchrest/model/properties.rb index 3c32b78..5e0e356 100644 --- a/lib/couchrest/model/properties.rb +++ b/lib/couchrest/model/properties.rb @@ -6,9 +6,9 @@ module CouchRest included do extlib_inheritable_accessor(:properties) unless self.respond_to?(:properties) - extlib_inheritable_accessor(:prop_by_name) unless self.respond_to?(:prop_by_name) + extlib_inheritable_accessor(:property_by_name) unless self.respond_to?(:property_by_name) self.properties ||= [] - self.prop_by_name ||= {} + self.property_by_name ||= {} raise "You can only mixin Properties in a class responding to [] and []=, if you tried to mixin CastedModel, make sure your class inherits from Hash or responds to the proper methods" unless (method_defined?(:[]) && method_defined?(:[]=)) end @@ -39,19 +39,12 @@ module CouchRest end # Store a casted value in the current instance of an attribute defined - # with a property. + # with a property and update dirty status def write_attribute(property, value) - prop = find_property!(property) - self[prop.to_s] = prop.is_a?(String) ? value : prop.cast(self, value) - end - - # write property, update dirty status - def write_attribute_dirty(property, value) prop = find_property!(property) value = prop.is_a?(String) ? value : prop.cast(self, value) - propname = prop.to_s - attribute_will_change!(propname) if use_dirty? && self[propname] != value - self[propname] = value + attribute_will_change!(prop.name) if use_dirty? && self[prop.name] != value + self[prop.name] = value end def []=(key,value) @@ -82,34 +75,47 @@ module CouchRest # Remove any protected and update all the rest. Any attributes # which do not have a property will simply be ignored. attrs = remove_protected_attributes(hash) - directly_set_attributes(attrs, :dirty => true) + directly_set_attributes(attrs) end alias :attributes= :update_attributes_without_saving # 'attributes' needed for Dirty alias :attributes :properties_with_values + def set_attributes(hash) + attrs = remove_protected_attributes(hash) + directly_set_attributes(attrs) + end + + protected + def find_property(property) - property.is_a?(Property) ? property : self.class.prop_by_name[property.to_s] + property.is_a?(Property) ? property : self.class.property_by_name[property.to_s] end # The following methods should be accessable by the Model::Base Class, but not by anything else! def apply_all_property_defaults return if self.respond_to?(:new?) && (new? == false) # TODO: cache the default object + # Never mark default options as dirty! + dirty, self.disable_dirty = self.disable_dirty, true self.class.properties.each do |property| write_attribute(property, property.default_value) end + self.disable_dirty = dirty end def prepare_all_attributes(doc = {}, options = {}) + self.disable_dirty = !!options[:directly_set_attributes] apply_all_property_defaults if options[:directly_set_attributes] directly_set_read_only_attributes(doc) else doc = remove_protected_attributes(doc) end - directly_set_attributes(doc) unless doc.nil? + res = doc.nil? ? doc : directly_set_attributes(doc) + self.disable_dirty = false + res end def find_property!(property) @@ -120,9 +126,8 @@ module CouchRest # Set all the attributes and return a hash with the attributes # that have not been accepted. - def directly_set_attributes(hash, options = {}) - self.disable_dirty = !options[:dirty] - ret = hash.reject do |attribute_name, attribute_value| + def directly_set_attributes(hash) + hash.reject do |attribute_name, attribute_value| if self.respond_to?("#{attribute_name}=") self.send("#{attribute_name}=", attribute_value) true @@ -133,8 +138,6 @@ module CouchRest false end end - self.disable_dirty = false - ret end def directly_set_read_only_attributes(hash) @@ -147,10 +150,6 @@ module CouchRest end end - def set_attributes(hash) - attrs = remove_protected_attributes(hash) - directly_set_attributes(attrs) - end module ClassMethods @@ -206,14 +205,14 @@ module CouchRest end type = [type] # inject as an array end - property = Property.new(name, type, options.merge(:use_dirty => use_dirty)) + property = Property.new(name, type, options) create_property_getter(property) create_property_setter(property) unless property.read_only == true if property.type_class.respond_to?(:validates_casted_model) validates_casted_model property.name end properties << property - prop_by_name[property.to_s] = property + property_by_name[property.to_s] = property property end @@ -247,7 +246,7 @@ module CouchRest property_name = property.name class_eval <<-EOS def #{property_name}=(value) - write_attribute_dirty('#{property_name}', value) + write_attribute('#{property_name}', value) end EOS diff --git a/lib/couchrest/model/property.rb b/lib/couchrest/model/property.rb index 783cc69..4af8864 100644 --- a/lib/couchrest/model/property.rb +++ b/lib/couchrest/model/property.rb @@ -4,7 +4,7 @@ module CouchRest::Model include ::CouchRest::Model::Typecast - attr_reader :name, :type, :type_class, :read_only, :alias, :default, :casted, :init_method, :use_dirty, :options + attr_reader :name, :type, :type_class, :read_only, :alias, :default, :casted, :init_method, :options # Attribute to define. # All Properties are assumed casted unless the type is nil. @@ -38,8 +38,8 @@ module CouchRest::Model end arr = value.collect { |data| cast_value(parent, data) } # allow casted_by calls to be passed up chain by wrapping in CastedArray - value = (use_dirty || type_class != String) ? CastedArray.new(arr, self) : arr - value.casted_by = parent if value.respond_to?(:casted_by) + value = CastedArray.new(arr, self) + value.casted_by = parent elsif (type == Object || type == Hash) && (value.class == Hash) # allow casted_by calls to be passed up chain by wrapping in CastedHash value = CouchRest::Model::CastedHash[value] @@ -94,7 +94,6 @@ module CouchRest::Model @alias = options.delete(:alias) if options[:alias] @default = options.delete(:default) unless options[:default].nil? @init_method = options[:init_method] ? options.delete(:init_method) : 'new' - @use_dirty = options.delete(:use_dirty) @options = options end diff --git a/spec/couchrest/dirty_spec.rb b/spec/couchrest/dirty_spec.rb index cf47aeb..990cb7a 100644 --- a/spec/couchrest/dirty_spec.rb +++ b/spec/couchrest/dirty_spec.rb @@ -14,88 +14,25 @@ class WithCastedModelMixin < Hash end class DummyModel < CouchRest::Model::Base - use_database TEST_SERVER.default_database - raise "Default DB not set" if TEST_SERVER.default_database.nil? + use_database DB + property :casted_attribute, WithCastedModelMixin + property :title, :default => 'Sample Title' property :details, Object, :default => { 'color' => 'blue' } property :keywords, [String], :default => ['default-keyword'] - property :sub_models do |child| - child.property :title + property :sub_models do + property :title end end -# set dirty configuration, return previous configuration setting -def set_dirty(value) - orig = nil - CouchRest::Model::Base.configure do |config| - orig = config.use_dirty - config.use_dirty = value - end - Card.instance_eval do - self.use_dirty = value - end - orig -end - -describe "With use_dirty(off)" do - - before(:all) do - @use_dirty_orig = set_dirty(false) - end - - # turn dirty back to default - after(:all) do - set_dirty(@use_dirty_orig) - end - - describe "changes" do - - it "should not respond to the changes method" do - @card = Card.new - @card.first_name = "andrew" - @card.changes.should == {} - end - - end - - describe "changed?" do - - it "should not record changes" do - @card = Card.new - @card.first_name = "andrew" - @card.changed?.should be_false - end - end - - describe "save" do - - it "should save unchanged records" do - @card = Card.create!(:first_name => "matt") - @card = Card.find(@card.id) - @card.database.should_receive(:save_doc).and_return({"ok" => true}) - @card.save - end - - end - -end - -describe "With use_dirty(on)" do - - before(:all) do - @use_dirty_orig = set_dirty(true) - end - - # turn dirty back to default - after(:all) do - set_dirty(@use_dirty_orig) - end +describe "Dirty" do describe "changes" do it "should return changes on an attribute" do @card = Card.new(:first_name => "matt") @card.first_name = "andrew" + @card.first_name_changed?.should be_true @card.changes.should == { "first_name" => ["matt", "andrew"] } end diff --git a/spec/couchrest/property_spec.rb b/spec/couchrest/property_spec.rb index 5d21079..a8c81b9 100644 --- a/spec/couchrest/property_spec.rb +++ b/spec/couchrest/property_spec.rb @@ -357,10 +357,10 @@ describe "Property Class" do property.cast(parent, ["2010-06-01", "2010-06-02"]).class.should eql(CouchRest::Model::CastedArray) end - it "should not set a CastedArray on array of Strings" do + it "should set a CastedArray on array of Strings" do property = CouchRest::Model::Property.new(:test, [String]) parent = mock("FooObject") - property.cast(parent, ["2010-06-01", "2010-06-02"]).class.should_not eql(CouchRest::Model::CastedArray) + property.cast(parent, ["2010-06-01", "2010-06-02"]).class.should eql(CouchRest::Model::CastedArray) end it "should raise and error if value is array when type is not" do diff --git a/spec/fixtures/more/card.rb b/spec/fixtures/more/card.rb index 7080ce9..7494b44 100644 --- a/spec/fixtures/more/card.rb +++ b/spec/fixtures/more/card.rb @@ -7,10 +7,10 @@ class Card < CouchRest::Model::Base property :last_name, :alias => :family_name property :read_only_value, :read_only => true property :cast_alias, Person, :alias => :calias + property :fg_color, :default => '#000' - timestamps! - + # Validation validates_presence_of :first_name