From 2ee92a533102cf87c6ef435f6bda25464ab18623 Mon Sep 17 00:00:00 2001 From: Will Leinweber Date: Thu, 2 Dec 2010 15:06:50 -0600 Subject: [PATCH] Removes suppport for ActiveModel::Dirty and ::AttributeMethods for performance reasons Removes commit d3331333194ba31ce1d26448cd542086af6ab662 --- history.txt | 2 - lib/couchrest/model/attributes.rb | 86 +++---------------- lib/couchrest/model/base.rb | 31 ++++--- lib/couchrest/model/casted_model.rb | 16 ++-- lib/couchrest/model/dirty.rb | 44 ---------- lib/couchrest/model/properties.rb | 54 ++++++++++-- lib/couchrest_model.rb | 1 - spec/couchrest/base_spec.rb | 80 +++++++++--------- spec/couchrest/dirty_spec.rb | 126 ---------------------------- spec/couchrest/property_spec.rb | 14 ---- 10 files changed, 123 insertions(+), 331 deletions(-) delete mode 100644 lib/couchrest/model/dirty.rb delete mode 100644 spec/couchrest/dirty_spec.rb diff --git a/history.txt b/history.txt index f05d44f..d7e157f 100644 --- a/history.txt +++ b/history.txt @@ -1,8 +1,6 @@ == Next Version * Major enhancements - * Dirty Tracking via ActiveModel - * ActiveModel Attribute Methods support * Minor enhancements * Fixing find("") issue (thanks epochwolf) diff --git a/lib/couchrest/model/attributes.rb b/lib/couchrest/model/attributes.rb index c2f2484..391cdb5 100644 --- a/lib/couchrest/model/attributes.rb +++ b/lib/couchrest/model/attributes.rb @@ -1,66 +1,17 @@ module CouchRest module Model - ReadOnlyPropertyError = Class.new(StandardError) - - # Attributes Suffixes provide methods from ActiveModel - # to hook into. See methods such as #attribute= and - # #attribute? for their implementation - AttributeMethodSuffixes = ['', '=', '?'] - module Attributes - extend ActiveSupport::Concern - - included do - include ActiveModel::AttributeMethods - attribute_method_suffix *AttributeMethodSuffixes - end - - module ClassMethods - def attributes - properties.map {|prop| prop.name} - end - end - - def initialize(*args) - self.class.attribute_method_suffix *AttributeMethodSuffixes - super - end - - def attributes - self.class.attributes - end - - ## Reads the attribute value. - # Assuming you have a property :title this would be called - # by `model_instance.title` - def attribute(name) - read_attribute(name) - end - - ## Sets the attribute value. - # Assuming you have a property :title this would be called - # by `model_instance.title = 'hello'` - def attribute=(name, value) - raise ReadOnlyPropertyError, 'read only property' if find_property!(name).read_only - write_attribute(name, value) - end - - ## Tests for both presence and truthiness of the attribute. - # Assuming you have a property :title # this would be called - # by `model_instance.title?` - def attribute?(name) - value = read_attribute(name) - !(value.nil? || value == false) - end ## Support for handling attributes - # + # # This would be better in the properties file, but due to scoping issues # this is not yet possible. + # + def prepare_all_attributes(doc = {}, options = {}) apply_all_property_defaults if options[:directly_set_attributes] - directly_set_read_only_attributes(doc) + directly_set_read_only_attributes(doc) else remove_protected_attributes(doc) end @@ -69,7 +20,7 @@ module CouchRest # Takes a hash as argument, and applies the values by using writer methods # for each key. It doesn't save the document at the end. Raises a NoMethodError if the corresponding methods are - # missing. In case of error, no attributes are changed. + # missing. In case of error, no attributes are changed. def update_attributes_without_saving(hash) # Remove any protected and update all the rest. Any attributes # which do not have a property will simply be ignored. @@ -78,26 +29,11 @@ module CouchRest end alias :attributes= :update_attributes_without_saving - def read_attribute(property) - prop = find_property!(property) - self[prop.to_s] - end - - def write_attribute(property, value) - prop = find_property!(property) - self[prop.to_s] = prop.cast(self, value) - end private - def read_only_attributes - properties.select { |prop| prop.read_only }.map { |prop| prop.name } - end - def directly_set_attributes(hash) - r_o_a = read_only_attributes hash.each do |attribute_name, attribute_value| - next if r_o_a.include? attribute_name if self.respond_to?("#{attribute_name}=") self.send("#{attribute_name}=", hash.delete(attribute_name)) end @@ -105,27 +41,27 @@ module CouchRest end def directly_set_read_only_attributes(hash) - r_o_a = read_only_attributes - property_list = attributes + property_list = self.properties.map{|p| p.name} hash.each do |attribute_name, attribute_value| - next unless r_o_a.include? attribute_name + next if self.respond_to?("#{attribute_name}=") if property_list.include?(attribute_name) write_attribute(attribute_name, hash.delete(attribute_name)) end end end - + def set_attributes(hash) attrs = remove_protected_attributes(hash) directly_set_attributes(attrs) end def check_properties_exist(attrs) - property_list = attributes + property_list = self.properties.map{|p| p.name} attrs.each do |attribute_name, attribute_value| raise NoMethodError, "Property #{attribute_name} not created" unless respond_to?("#{attribute_name}=") or property_list.include?(attribute_name) - end + end end + end end end diff --git a/lib/couchrest/model/base.rb b/lib/couchrest/model/base.rb index 391f3b0..2a5c602 100644 --- a/lib/couchrest/model/base.rb +++ b/lib/couchrest/model/base.rb @@ -6,7 +6,7 @@ module CouchRest include CouchRest::Model::Persistence include CouchRest::Model::Callbacks - include CouchRest::Model::DocumentQueries + include CouchRest::Model::DocumentQueries include CouchRest::Model::Views include CouchRest::Model::DesignDoc include CouchRest::Model::ExtendedAttachments @@ -16,12 +16,11 @@ module CouchRest include CouchRest::Model::Attributes include CouchRest::Model::Associations include CouchRest::Model::Validations - include CouchRest::Model::Dirty def self.subclasses @subclasses ||= [] end - + def self.inherited(subklass) super subklass.send(:include, CouchRest::Model::Properties) @@ -35,16 +34,16 @@ module CouchRest EOS subclasses << subklass end - + # Accessors attr_accessor :casted_by - + # Instantiate a new CouchRest::Model::Base by preparing all properties # using the provided document hash. # # Options supported: - # + # # * :directly_set_attributes: true when data comes directly from database # def initialize(doc = {}, options = {}) @@ -55,8 +54,8 @@ module CouchRest end after_initialize if respond_to?(:after_initialize) end - - + + # Temp solution to make the view_by methods available def self.method_missing(m, *args, &block) if has_view?(m) @@ -70,9 +69,9 @@ module CouchRest end super 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 @@ -80,14 +79,14 @@ module CouchRest return self if base_doc? @casted_by.base_doc end - + # Checks if we're the top document def base_doc? !@casted_by end - + ## Compatibility with ActiveSupport and older frameworks - + # Hack so that CouchRest::Document, which descends from Hash, # doesn't appear to Rails routing as a Hash of options def is_a?(klass) @@ -99,14 +98,14 @@ module CouchRest def persisted? !new? end - + def to_key - new? ? nil : [id] + new? ? nil : [id] end alias :to_param :id alias :new_record? :new? alias :new_document? :new? - end + end end end diff --git a/lib/couchrest/model/casted_model.rb b/lib/couchrest/model/casted_model.rb index a44f987..9ef1603 100644 --- a/lib/couchrest/model/casted_model.rb +++ b/lib/couchrest/model/casted_model.rb @@ -1,6 +1,6 @@ module CouchRest::Model module CastedModel - + extend ActiveSupport::Concern included do @@ -12,28 +12,28 @@ module CouchRest::Model include CouchRest::Model::Validations attr_accessor :casted_by end - + def initialize(keys = {}) raise StandardError unless self.is_a? Hash prepare_all_attributes(keys) super() end - + def []= 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? @@ -53,12 +53,12 @@ module CouchRest::Model end alias :to_key :id alias :to_param :id - + # Sets the attributes from a hash def update_attributes_without_saving(hash) hash.each do |k, v| raise NoMethodError, "#{k}= method not available, use property :#{k}" unless self.respond_to?("#{k}=") - end + end hash.each do |k, v| self.send("#{k}=",v) end diff --git a/lib/couchrest/model/dirty.rb b/lib/couchrest/model/dirty.rb deleted file mode 100644 index ae5b461..0000000 --- a/lib/couchrest/model/dirty.rb +++ /dev/null @@ -1,44 +0,0 @@ -# encoding: utf-8 -require 'active_model/dirty' - -module CouchRest #:nodoc: - module Model #:nodoc: - - # Dirty Tracking support via ActiveModel - # mixin methods include: - # #changed?, #changed, #changes, #previous_changes - # #_changed?, #_change, - # #reset_!, #_will_change!, - # and #_was - # - # Please see the specs or the documentation of - # ActiveModel::Dirty for more information - module Dirty - extend ActiveSupport::Concern - - included do - include ActiveModel::Dirty - after_save :clear_changed_attributes - end - - def initialize(*args) - super - @changed_attributes.clear if @changed_attributes - end - - def write_attribute(name, value) - meth = :"#{name}_will_change!" - __send__ meth if respond_to? meth - super - end - - private - - def clear_changed_attributes - @previously_changed = changes - @changed_attributes.clear - true - end - end - end -end diff --git a/lib/couchrest/model/properties.rb b/lib/couchrest/model/properties.rb index 4f5701f..399673e 100644 --- a/lib/couchrest/model/properties.rb +++ b/lib/couchrest/model/properties.rb @@ -1,5 +1,4 @@ # encoding: utf-8 -require 'set' module CouchRest module Model module Properties @@ -23,6 +22,16 @@ module CouchRest self.class.properties end + def read_attribute(property) + prop = find_property!(property) + self[prop.to_s] + end + + def write_attribute(property, value) + prop = find_property!(property) + self[prop.to_s] = prop.cast(self, value) + end + def apply_all_property_defaults return if self.respond_to?(:new?) && (new? == false) # TODO: cache the default object @@ -85,7 +94,8 @@ module CouchRest type = [type] # inject as an array end property = Property.new(name, type, options) - create_property_alias(property) if property.alias + 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 @@ -93,15 +103,49 @@ module CouchRest property end - def create_property_alias(property) + # defines the getter for the property (and optional aliases) + def create_property_getter(property) + # meth = property.name class_eval <<-EOS, __FILE__, __LINE__ + 1 - def #{property.alias.to_s} - #{property.name} + def #{property.name} + read_attribute('#{property.name}') end EOS + + if ['boolean', TrueClass.to_s.downcase].include?(property.type.to_s.downcase) + class_eval <<-EOS, __FILE__, __LINE__ + def #{property.name}? + value = read_attribute('#{property.name}') + !(value.nil? || value == false) + end + EOS + end + + if property.alias + class_eval <<-EOS, __FILE__, __LINE__ + 1 + alias #{property.alias.to_sym} #{property.name.to_sym} + EOS + end + end + + # defines the setter for the property (and optional aliases) + def create_property_setter(property) + property_name = property.name + class_eval <<-EOS + def #{property_name}=(value) + write_attribute('#{property_name}', value) + end + EOS + + if property.alias + class_eval <<-EOS + alias #{property.alias.to_sym}= #{property_name.to_sym}= + EOS + end end end # module ClassMethods + end end end diff --git a/lib/couchrest_model.rb b/lib/couchrest_model.rb index e69bbf4..0a3d657 100644 --- a/lib/couchrest_model.rb +++ b/lib/couchrest_model.rb @@ -48,7 +48,6 @@ require "couchrest/model/collection" require "couchrest/model/attribute_protection" require "couchrest/model/attributes" require "couchrest/model/associations" -require "couchrest/model/dirty" # Monkey patches applied to couchrest require "couchrest/model/support/couchrest" diff --git a/spec/couchrest/base_spec.rb b/spec/couchrest/base_spec.rb index 35f182b..cd5fd4d 100644 --- a/spec/couchrest/base_spec.rb +++ b/spec/couchrest/base_spec.rb @@ -8,23 +8,23 @@ require File.join(FIXTURE_PATH, 'more', 'card') require File.join(FIXTURE_PATH, 'base') describe "Model Base" do - + before(:each) do @obj = WithDefaultValues.new end - + describe "instance database connection" do it "should use the default database" do @obj.database.name.should == 'couchrest-model-test' end - + it "should override the default db" do @obj.database = TEST_SERVER.database!('couchrest-extendedmodel-test') @obj.database.name.should == 'couchrest-extendedmodel-test' @obj.database.delete! end end - + describe "a new model" do it "should be a new document" do @obj = Basic.new @@ -39,10 +39,10 @@ describe "Model Base" do @obj.should == { 'couchrest-type' => 'Basic' } end end - + describe "ActiveModel compatability Basic" do - before(:each) do + before(:each) do @obj = Basic.new(nil) end @@ -86,7 +86,7 @@ describe "Model Base" do context "when the document is not new" do it "returns id" do @obj.save - @obj.persisted?.should == true + @obj.persisted?.should == true end end end @@ -100,7 +100,7 @@ describe "Model Base" do end - + describe "update attributes without saving" do before(:each) do a = Article.get "big-bad-danger" rescue nil @@ -134,22 +134,22 @@ describe "Model Base" do @art.attributes = {'date' => Time.now, :title => "something else"} @art['title'].should == "something else" end - + it "should not flip out if an attribute= method is missing and ignore it" do lambda { @art.update_attributes_without_saving('slug' => "new-slug", :title => "super danger") }.should_not raise_error @art.slug.should == "big-bad-danger" end - + #it "should not change other attributes if there is an error" do # lambda { - # @art.update_attributes_without_saving('slug' => "new-slug", :title => "super danger") + # @art.update_attributes_without_saving('slug' => "new-slug", :title => "super danger") # }.should raise_error # @art['title'].should == "big bad danger" #end end - + describe "update attributes" do before(:each) do a = Article.get "big-bad-danger" rescue nil @@ -164,7 +164,7 @@ describe "Model Base" do loaded['title'].should == "super danger" end end - + describe "with default" do it "should have the default value set at initalization" do @obj.preset.should == {:right => 10, :top_align => false} @@ -173,23 +173,23 @@ describe "Model Base" do it "should have the default false value explicitly assigned" do @obj.default_false.should == false end - + it "should automatically call a proc default at initialization" do @obj.set_by_proc.should be_an_instance_of(Time) @obj.set_by_proc.should == @obj.set_by_proc @obj.set_by_proc.should < Time.now end - + it "should let you overwrite the default values" do obj = WithDefaultValues.new(:preset => 'test') obj.preset = 'test' end - + it "should work with a default empty array" do obj = WithDefaultValues.new(:tags => ['spec']) obj.tags.should == ['spec'] end - + it "should set default value of read-only property" do obj = WithDefaultValues.new obj.read_only_with_default.should == 'generic' @@ -207,7 +207,7 @@ describe "Model Base" do obj.tags.should == ['spec'] end end - + describe "a doc with template values (CR::Model spec)" do before(:all) do WithTemplateAndUniqueID.all.map{|o| o.destroy} @@ -228,8 +228,8 @@ describe "Model Base" do tmpl2_reloaded.preset.should == 'not_value' end end - - + + describe "finding all instances of a model" do before(:all) do WithTemplateAndUniqueID.req_design_doc_refresh @@ -246,32 +246,32 @@ describe "Model Base" do d['views']['all']['map'].should include('WithTemplateAndUniqueID') end it "should find all" do - rs = WithTemplateAndUniqueID.all + rs = WithTemplateAndUniqueID.all rs.length.should == 4 end end - + describe "counting all instances of a model" do before(:each) do @db = reset_test_db! WithTemplateAndUniqueID.req_design_doc_refresh end - + it ".count should return 0 if there are no docuemtns" do WithTemplateAndUniqueID.count.should == 0 end - + it ".count should return the number of documents" do WithTemplateAndUniqueID.new('important-field' => '1').save WithTemplateAndUniqueID.new('important-field' => '2').save WithTemplateAndUniqueID.new('important-field' => '3').save - + WithTemplateAndUniqueID.count.should == 3 end end - + describe "finding the first instance of a model" do - before(:each) do + before(:each) do @db = reset_test_db! # WithTemplateAndUniqueID.req_design_doc_refresh # Removed by Sam Lown, design doc should be loaded automatically WithTemplateAndUniqueID.new('important-field' => '1').save @@ -309,7 +309,7 @@ describe "Model Base" do WithTemplateAndUniqueID.design_doc['_rev'].should eql(rev) end end - + describe "getting a model with a subobject field" do before(:all) do course_doc = { @@ -332,7 +332,7 @@ describe "Model Base" do @course['ends_at'].should == Time.parse("2008/12/19 13:00:00 +0800") end end - + describe "timestamping" do before(:each) do oldart = Article.get "saving-this" rescue nil @@ -340,7 +340,7 @@ describe "Model Base" do @art = Article.new(:title => "Saving this") @art.save end - + it "should define the updated_at and created_at getters and set the values" do @obj.save obj = WithDefaultValues.get(@obj.id) @@ -349,15 +349,15 @@ describe "Model Base" do obj.updated_at.should be_an_instance_of(Time) obj.created_at.to_s.should == @obj.updated_at.to_s end - + it "should not change created_at on update" do - 2.times do + 2.times do lambda do @art.save end.should_not change(@art, :created_at) end end - + it "should set the time on create" do (Time.now - @art.created_at).should < 2 foundart = Article.get @art.id @@ -368,7 +368,7 @@ describe "Model Base" do @art.created_at.should < @art.updated_at end end - + describe "getter and setter methods" do it "should try to call the arg= method before setting :arg in the hash" do @doc = WithGetterAndSetterMethods.new(:arg => "foo") @@ -384,41 +384,41 @@ describe "Model Base" do @doc['some_value'].should eql('value') end end - + describe "recursive validation on a model" do before :each do reset_test_db! @cat = Cat.new(:name => 'Sockington') end - + it "should not save if a nested casted model is invalid" do @cat.favorite_toy = CatToy.new @cat.should_not be_valid @cat.save.should be_false lambda{@cat.save!}.should raise_error end - + it "should save when nested casted model is valid" do @cat.favorite_toy = CatToy.new(:name => 'Squeaky') @cat.should be_valid @cat.save.should be_true lambda{@cat.save!}.should_not raise_error end - + it "should not save when nested collection contains an invalid casted model" do @cat.toys = [CatToy.new(:name => 'Feather'), CatToy.new] @cat.should_not be_valid @cat.save.should be_false lambda{@cat.save!}.should raise_error end - + it "should save when nested collection contains valid casted models" do @cat.toys = [CatToy.new(:name => 'feather'), CatToy.new(:name => 'ball-o-twine')] @cat.should be_valid @cat.save.should be_true lambda{@cat.save!}.should_not raise_error end - + it "should not fail if the nested casted model doesn't have validation" do Cat.property :trainer, Person Cat.validates_presence_of :name diff --git a/spec/couchrest/dirty_spec.rb b/spec/couchrest/dirty_spec.rb deleted file mode 100644 index c7c2f10..0000000 --- a/spec/couchrest/dirty_spec.rb +++ /dev/null @@ -1,126 +0,0 @@ -require File.expand_path("../../spec_helper", __FILE__) - -class DirtyModel < CouchRest::Model::Base - use_database TEST_SERVER.default_database - property :name - property :color - validates_presence_of :name -end - -describe 'Dirty Tracking', '#changed?' do - before(:each) do - @dm = DirtyModel.new - @dm.name = 'will' - end - - it 'brand new models should not be changed by default' do - DirtyModel.new.should_not be_changed - end - - it 'save should reset changed?' do - @dm.should be_changed - @dm.save - @dm.should_not be_changed - end - - it 'save! should reset changed?' do - @dm.should be_changed - @dm.save! - @dm.should_not be_changed - end - - it 'a failed save should preserve changed?' do - @dm.name = '' - @dm.should be_changed - @dm.save.should be_false - @dm.should be_changed - end - - it 'should be true if there have been changes' do - @dm.name = 'not will' - @dm.should be_changed - end -end - -describe 'Dirty Tracking', '#changed' do - it 'should be an array of the changed attributes' do - dm = DirtyModel.new - dm.changed.should == [] - dm.name = 'will' - dm.changed.should == ['name'] - dm.color = 'red' - dm.changed.should =~ ['name', 'color'] - end -end - -describe 'Dirty Tracking', '#changes' do - it 'should be a Map of changed attrs => [original value, new value]' do - dm = DirtyModel.new(:name => 'will', :color => 'red') - dm.save! - dm.should_not be_changed - - dm.name = 'william' - dm.color = 'blue' - - dm.changes.should == { 'name' => ['will', 'william'], 'color' => ['red', 'blue'] } - end -end - -describe 'Dirty Tracking', '#previous_changes' do - it 'should store the previous changes after a save' do - dm = DirtyModel.new(:name => 'will', :color => 'red') - dm.save! - dm.should_not be_changed - - dm.name = 'william' - dm.save! - - dm.previous_changes.should == { 'name' => ['will', 'william'] } - end -end - -describe 'Dirty Tracking', 'attribute methods' do - before(:each) do - @dm = DirtyModel.new(:name => 'will', :color => 'red') - @dm.save! - end - - describe '#_changed?' do - it 'it should know if a specific property was changed' do - @dm.name = 'william' - @dm.should be_name_changed - @dm.should_not be_color_changed - end - end - - describe 'Dirty Tracking', '#_change' do - it 'should be an array of [original value, current value]' do - @dm.name = 'william' - @dm.name_change.should == ['will', 'william'] - end - end - - describe 'Dirty Tracking', '#_was' do - it 'should return what the attribute was' do - @dm.name = 'william' - @dm.name_was.should == 'will' - end - end - - describe 'Dirty Tracking', '#reset_!' do - it 'should reset the attribute to what it was' do - @dm.name = 'william' - - @dm.reset_name! - @dm.name.should == 'will' - end - end - - describe 'Dirty Tracking', '#_will_change!' do - it 'should manually mark the attribute as changed' do - @dm.should_not be_name_changed - @dm.name_will_change! - @dm.should be_name_changed - end - end -end diff --git a/spec/couchrest/property_spec.rb b/spec/couchrest/property_spec.rb index 7e59be1..95352a9 100644 --- a/spec/couchrest/property_spec.rb +++ b/spec/couchrest/property_spec.rb @@ -9,20 +9,6 @@ require File.join(FIXTURE_PATH, 'more', 'event') require File.join(FIXTURE_PATH, 'more', 'user') require File.join(FIXTURE_PATH, 'more', 'course') -describe 'Attributes' do - class AttrDoc < CouchRest::Model::Base - property :one - property :two - end - - it '.attributes should have an array of attribute names' do - AttrDoc.attributes.should =~ ['two', 'one'] - end - - it '#attributes should have an array of attribute names' do - AttrDoc.new.attributes.should =~ ['two', 'one'] - end -end describe "Model properties" do