diff --git a/Gemfile.lock b/Gemfile.lock index 312c498..8ecf864 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -12,9 +12,9 @@ GEM remote: http://rubygems.org/ specs: abstract (1.0.0) - actionpack (3.0.5) - activemodel (= 3.0.5) - activesupport (= 3.0.5) + actionpack (3.0.4) + activemodel (= 3.0.4) + activesupport (= 3.0.4) builder (~> 2.1.2) erubis (~> 2.6.6) i18n (~> 0.4) @@ -22,11 +22,11 @@ GEM rack-mount (~> 0.6.13) rack-test (~> 0.5.7) tzinfo (~> 0.3.23) - activemodel (3.0.5) - activesupport (= 3.0.5) + activemodel (3.0.4) + activesupport (= 3.0.4) builder (~> 2.1.2) i18n (~> 0.4) - activesupport (3.0.5) + activesupport (3.0.4) builder (2.1.2) couchrest (1.0.1) json (>= 1.4.6) @@ -43,9 +43,9 @@ GEM rack (>= 1.0.0) rack-test (0.5.7) rack (>= 1.0) - railties (3.0.5) - actionpack (= 3.0.5) - activesupport (= 3.0.5) + railties (3.0.4) + actionpack (= 3.0.4) + activesupport (= 3.0.4) rake (>= 0.8.7) thor (~> 0.14.4) rake (0.8.7) diff --git a/b/couchrest.rb b/b/couchrest.rb index 698a6d4..7ab8fb1 100644 --- a/b/couchrest.rb +++ b/b/couchrest.rb @@ -20,13 +20,27 @@ class BenchmarkModel < CouchRest::Model::Base property :casted_list, [BenchmarkCasted] end -begin - n = 50000 +# 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 + BenchmarkModel.instance_eval do + self.use_dirty = value + end + orig +end + +def run_benchmark + n = 50000 # property operation count + db_n = 1000 # database operation count b = BenchmarkModel.new - Benchmark.bm(25) do |x| + Benchmark.bm(30) do |x| - # assigning + # property assigning x.report("assign string:") do n.times { b.string = "test" } @@ -44,7 +58,7 @@ begin n.times { b.casted_list = [{ 'name' => 'test' }] } end - # reading + # property reading x.report("read string") do n.times { b.string } @@ -62,13 +76,32 @@ begin n.times { b.casted_list } end - # db writing if ENV['BENCHMARK_DB'] - x.report("write record") do - # need to make change before it will save - n.times { b.string = "test#{n}"; b.save } + # db writing + x.report("write changed record to db") do + db_n.times { |i| b.string = "test#{i}"; b.save } end + + x.report("write unchanged record to db") do + db_n.times { b.save } + end + + # db reading + x.report("read record from db") do + db_n.times { BenchmarkModel.find(b.id) } + end + end end end + +begin + puts "with use_dirty true" + set_dirty(true) + run_benchmark + + puts "\nwith use_dirty false" + set_dirty(false) + run_benchmark +end diff --git a/lib/couchrest/model/base.rb b/lib/couchrest/model/base.rb index 9d47d1b..cb3f3ab 100644 --- a/lib/couchrest/model/base.rb +++ b/lib/couchrest/model/base.rb @@ -17,6 +17,7 @@ module CouchRest include CouchRest::Model::Associations include CouchRest::Model::Validations include CouchRest::Model::Dirty + include CouchRest::Model::CastedBy def self.subclasses @subclasses ||= [] @@ -25,6 +26,7 @@ module CouchRest def self.inherited(subklass) super subklass.send(:include, CouchRest::Model::Properties) + subklass.class_eval <<-EOS, __FILE__, __LINE__ + 1 def self.inherited(subklass) super @@ -72,16 +74,9 @@ module CouchRest end ### instance methods - - # Gets a reference to the actual document in the DB - # Calls up to the next document if there is one, - # Otherwise we're at the top and we return self - def base_doc - return self if base_doc? - @casted_by.base_doc - end - + # Checks if we're the top document + # (overrides base_doc? in casted_by.rb) def base_doc? !@casted_by end diff --git a/lib/couchrest/model/casted_by.rb b/lib/couchrest/model/casted_by.rb new file mode 100644 index 0000000..0f192d6 --- /dev/null +++ b/lib/couchrest/model/casted_by.rb @@ -0,0 +1,23 @@ + +module CouchRest::Model + module CastedBy + extend ActiveSupport::Concern + included do + self.send(:attr_accessor, :casted_by) + end + + # Gets a reference to the actual document in the DB + # Calls up to the next document if there is one, + # Otherwise we're at the top and we return self + def base_doc + return self if base_doc? + @casted_by ? @casted_by.base_doc : nil + end + + # Checks if we're the top document + def base_doc? + false + end + + end +end diff --git a/lib/couchrest/model/casted_hash.rb b/lib/couchrest/model/casted_hash.rb index 737c91b..e1d56af 100644 --- a/lib/couchrest/model/casted_hash.rb +++ b/lib/couchrest/model/casted_hash.rb @@ -7,6 +7,7 @@ module CouchRest::Model attr_accessor :casted_by def []= index, obj + return super(index, obj) unless use_dirty? couchrest_parent_will_change! if obj != self[index] super(index, obj) end diff --git a/lib/couchrest/model/casted_model.rb b/lib/couchrest/model/casted_model.rb index 19b6fc9..678c8ad 100644 --- a/lib/couchrest/model/casted_model.rb +++ b/lib/couchrest/model/casted_model.rb @@ -21,7 +21,7 @@ module CouchRest::Model end def []= key, value - couchrest_attribute_will_change!(key) unless self[key] == value + couchrest_attribute_will_change!(key) if use_dirty && self[key] != value super(key.to_s, value) end diff --git a/lib/couchrest/model/configuration.rb b/lib/couchrest/model/configuration.rb index 765a4b1..25ebfcb 100644 --- a/lib/couchrest/model/configuration.rb +++ b/lib/couchrest/model/configuration.rb @@ -10,10 +10,12 @@ module CouchRest included do add_config :model_type_key add_config :mass_assign_any_attribute - + add_config :use_dirty + configure do |config| config.model_type_key = 'couchrest-type' # 'model'? config.mass_assign_any_attribute = false + config.use_dirty = true end end diff --git a/lib/couchrest/model/dirty.rb b/lib/couchrest/model/dirty.rb index 0000fa0..3d422c9 100644 --- a/lib/couchrest/model/dirty.rb +++ b/lib/couchrest/model/dirty.rb @@ -10,14 +10,24 @@ module CouchRest # This applies to both Model::Base and Model::CastedModel module Dirty extend ActiveSupport::Concern + include CouchRest::Model::CastedBy # needed for base_doc + include ActiveModel::Dirty + + def use_dirty? + bdoc = base_doc + bdoc && !bdoc.disable_dirty && bdoc.use_dirty + end included do - include ActiveModel::Dirty + # internal dirty setting - overrides global setting. + # this is used to temporarily disable dirty tracking when setting + # attributes directly, for performance reasons. + self.send(:attr_accessor, :disable_dirty) end def couchrest_attribute_will_change!(attr) - return if attr.nil? - self.send("#{attr}_will_change!") + return if attr.nil? || !use_dirty? + attribute_will_change!(attr) couchrest_parent_will_change! end @@ -29,7 +39,7 @@ module CouchRest # return the attribute name this object is referenced by in the parent def casted_by_attribute - return @casted_by_attribute if @casted_by_attribute_set + return @casted_by_attribute if @casted_by_attribute attr = @casted_by.attributes @casted_by_attribute = attr.keys.detect { |k| attr[k] == self } end diff --git a/lib/couchrest/model/extended_attachments.rb b/lib/couchrest/model/extended_attachments.rb index c2a5ffc..d36488f 100644 --- a/lib/couchrest/model/extended_attachments.rb +++ b/lib/couchrest/model/extended_attachments.rb @@ -2,11 +2,6 @@ module CouchRest module Model module ExtendedAttachments extend ActiveSupport::Concern - include ActiveModel::Dirty - included do - # for _attachments_will_change! - define_attribute_methods [:_attachments] - end # Add a file attachment to the current document. Expects # :file and :name to be included in the arguments. @@ -41,8 +36,10 @@ module CouchRest # deletes a file attachment from the current doc def delete_attachment(attachment_name) return unless attachments - _attachments_will_change! if attachments.include?(attachment_name) - attachments.delete attachment_name + if attachments.include?(attachment_name) + attribute_will_change!("_attachments") + attachments.delete attachment_name + end end # returns true if attachment_name exists @@ -74,7 +71,7 @@ module CouchRest content_type = args[:content_type] ? args[:content_type] : get_mime_type(args[:file].path) content_type ||= (get_mime_type(args[:name]) || 'text/plain') - _attachments_will_change! + attribute_will_change!("_attachments") attachments[args[:name]] = { 'content_type' => content_type, 'data' => args[:file].read diff --git a/lib/couchrest/model/persistence.rb b/lib/couchrest/model/persistence.rb index f4c7970..fc1fb4f 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 unless self.changed? + return true if use_dirty? && !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 a065092..152db8a 100644 --- a/lib/couchrest/model/properties.rb +++ b/lib/couchrest/model/properties.rb @@ -3,7 +3,6 @@ module CouchRest module Model module Properties extend ActiveSupport::Concern - include ActiveModel::Dirty included do extlib_inheritable_accessor(:properties) unless self.respond_to?(:properties) @@ -48,11 +47,14 @@ module CouchRest def write_attribute_dirty(property, value) prop = find_property!(property) value = prop.is_a?(String) ? value : prop.cast(self, value) - self.send("#{prop}_will_change!") unless self[prop.to_s] == value - write_attribute(property, value) + propname = prop.to_s + attribute_will_change!(propname) if use_dirty? && self[propname] != value + self[propname] = value end def []=(key,value) + return super(key,value) unless use_dirty? + has_changes = self.changed? if !has_changes && self.respond_to?(:get_unique_id) check_id_change = true @@ -82,14 +84,8 @@ module CouchRest end alias :attributes= :update_attributes_without_saving - # needed for Dirty - def attributes - ret = {} - self.class.properties.each do |property| - ret[property.name] = read_attribute(property) - end - ret - end + # 'attributes' needed for Dirty + alias :attributes :properties_with_values def find_property(property) property.is_a?(Property) ? property : self.class.properties.detect {|p| p.to_s == property.to_s} @@ -123,18 +119,10 @@ module CouchRest # Set all the attributes and return a hash with the attributes # that have not been accepted. def directly_set_attributes(hash, options = {}) - hash.reject do |attribute_name, attribute_value| + self.disable_dirty = !options[:dirty] + ret = hash.reject do |attribute_name, attribute_value| if self.respond_to?("#{attribute_name}=") - if find_property(attribute_name) - if options[:dirty] - self.write_attribute_dirty(attribute_name, attribute_value) - else - # set attribute without updating dirty status - self.write_attribute(attribute_name, attribute_value) - end - else - self.send("#{attribute_name}=", attribute_value) - end + self.send("#{attribute_name}=", attribute_value) true elsif mass_assign_any_attribute # config option self[attribute_name] = attribute_value @@ -143,6 +131,8 @@ module CouchRest false end end + self.disable_dirty = false + ret end def directly_set_read_only_attributes(hash) diff --git a/lib/couchrest_model.rb b/lib/couchrest_model.rb index 4d3e7c6..2a9850d 100644 --- a/lib/couchrest_model.rb +++ b/lib/couchrest_model.rb @@ -27,10 +27,11 @@ require 'couchrest/model' require 'couchrest/model/errors' require "couchrest/model/persistence" require "couchrest/model/typecast" +require "couchrest/model/casted_by" +require "couchrest/model/dirty" require "couchrest/model/property" require "couchrest/model/property_protection" require "couchrest/model/properties" -require "couchrest/model/dirty" require "couchrest/model/casted_array" require "couchrest/model/casted_hash" require "couchrest/model/casted_model" @@ -43,7 +44,7 @@ require "couchrest/model/extended_attachments" require "couchrest/model/class_proxy" require "couchrest/model/collection" require "couchrest/model/associations" -require "couchrest/model/configuration" +require 'couchrest/model/configuration' # Monkey patches applied to couchrest require "couchrest/model/support/couchrest" @@ -52,7 +53,6 @@ require "couchrest/model/support/hash" # Base libraries require "couchrest/model/casted_model" require "couchrest/model/base" - # Add rails support *after* everything has loaded require "couchrest/railtie" diff --git a/spec/couchrest/dirty_spec.rb b/spec/couchrest/dirty_spec.rb index 9184edc..098fa70 100644 --- a/spec/couchrest/dirty_spec.rb +++ b/spec/couchrest/dirty_spec.rb @@ -11,7 +11,7 @@ class WithCastedModelMixin < Hash property :name property :details, Object, :default => {} property :casted_attribute, WithCastedModelMixin -end +end class DummyModel < CouchRest::Model::Base use_database TEST_SERVER.default_database @@ -24,8 +24,72 @@ class DummyModel < CouchRest::Model::Base 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 "Dirty" do +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 "changes" do