From d41c7c96da99b4d3b937cfb2f3be3e2f7c250500 Mon Sep 17 00:00:00 2001 From: Will Leinweber Date: Fri, 4 Dec 2009 01:58:07 -0600 Subject: [PATCH] BUGFIX: attribute protection Fixes bug where documents recreated from the database were being stripped of their protected attributes when instantiated Signed-off-by: Marcos Tapajos --- lib/couchrest/mixins/collection.rb | 2 +- lib/couchrest/mixins/document_queries.rb | 8 +-- lib/couchrest/mixins/views.rb | 6 +- lib/couchrest/more/extended_document.rb | 29 +++++++--- .../more/attribute_protection_spec.rb | 56 +++++++++++++++++++ 5 files changed, 84 insertions(+), 17 deletions(-) diff --git a/lib/couchrest/mixins/collection.rb b/lib/couchrest/mixins/collection.rb index b248825..696dae7 100644 --- a/lib/couchrest/mixins/collection.rb +++ b/lib/couchrest/mixins/collection.rb @@ -183,7 +183,7 @@ module CouchRest if @container_class.nil? results else - results['rows'].collect { |row| @container_class.new(row['doc']) } unless results['rows'].nil? + results['rows'].collect { |row| @container_class.create_from_database(row['doc']) } unless results['rows'].nil? end end diff --git a/lib/couchrest/mixins/document_queries.rb b/lib/couchrest/mixins/document_queries.rb index d7c8a11..b19442e 100644 --- a/lib/couchrest/mixins/document_queries.rb +++ b/lib/couchrest/mixins/document_queries.rb @@ -51,11 +51,9 @@ module CouchRest # db:: optional option to pass a custom database to use def get(id, db = database) begin - doc = db.get id + get!(id, db) rescue nil - else - new(doc) end end @@ -72,11 +70,11 @@ module CouchRest # db:: optional option to pass a custom database to use def get!(id, db = database) doc = db.get id - new(doc) + create_from_database(doc) end end end end -end \ No newline at end of file +end diff --git a/lib/couchrest/mixins/views.rb b/lib/couchrest/mixins/views.rb index de530e3..6c95066 100644 --- a/lib/couchrest/mixins/views.rb +++ b/lib/couchrest/mixins/views.rb @@ -137,13 +137,13 @@ module CouchRest collection_proxy_for(design_doc, name, opts.merge({:include_docs => true})) else view = fetch_view db, name, opts.merge({:include_docs => true}), &block - view['rows'].collect{|r|new(r['doc'])} if view['rows'] + view['rows'].collect{|r|create_from_database(r['doc'])} if view['rows'] end rescue # fallback for old versions of couchdb that don't # have include_docs support view = fetch_view(db, name, opts, &block) - view['rows'].collect{|r|new(db.get(r['id']))} if view['rows'] + view['rows'].collect{|r|create_from_database(db.get(r['id']))} if view['rows'] end end end @@ -170,4 +170,4 @@ module CouchRest end end -end \ No newline at end of file +end diff --git a/lib/couchrest/more/extended_document.rb b/lib/couchrest/more/extended_document.rb index 73c1b17..0c26240 100644 --- a/lib/couchrest/more/extended_document.rb +++ b/lib/couchrest/more/extended_document.rb @@ -38,11 +38,20 @@ module CouchRest define_callbacks :save, "result == :halt" define_callbacks :update, "result == :halt" define_callbacks :destroy, "result == :halt" + + # Creates a new instance, bypassing attribute protection + # + # ==== Returns + # a document instance + def self.create_from_database(passed_keys={}) + new(passed_keys, :directly_set_attributes => true) + end - def initialize(passed_keys={}) + def initialize(passed_keys={}, options={}) apply_defaults # defined in CouchRest::Mixins::Properties - set_attributes(passed_keys) unless passed_keys.nil? - super + remove_protected_attributes(passed_keys) unless options[:directly_set_attributes] + directly_set_attributes(passed_keys) unless passed_keys.nil? + super(passed_keys) cast_keys # defined in CouchRest::Mixins::Properties unless self['_id'] && self['_rev'] self['couchrest-type'] = self.class.to_s @@ -281,14 +290,18 @@ module CouchRest raise NoMethodError, "#{attribute_name}= method not available, use property :#{attribute_name}" unless self.respond_to?("#{attribute_name}=") end end - - def set_attributes(hash) - attrs = remove_protected_attributes(hash) - attrs.each do |attribute_name, attribute_value| + + def directly_set_attributes(hash) + hash.each do |attribute_name, attribute_value| if self.respond_to?("#{attribute_name}=") - self.send("#{attribute_name}=", attrs.delete(attribute_name)) + self.send("#{attribute_name}=", hash.delete(attribute_name)) end end + end + + def set_attributes(hash) + attrs = remove_protected_attributes(hash) + directly_set_attributes(attrs) end end end diff --git a/spec/couchrest/more/attribute_protection_spec.rb b/spec/couchrest/more/attribute_protection_spec.rb index 3063531..f773167 100644 --- a/spec/couchrest/more/attribute_protection_spec.rb +++ b/spec/couchrest/more/attribute_protection_spec.rb @@ -21,6 +21,17 @@ describe "ExtendedDocument", "no declarations" do user.name.should == "will" user.phone.should == "555-5555" end + + it "should recreate from the database properly" do + user = NoProtection.new + user.name = "will" + user.phone = "555-5555" + user.save! + + user = NoProtection.get(user.id) + user.name.should == "will" + user.phone.should == "555-5555" + end end describe "ExtendedDocument", "accessible flag" do @@ -92,3 +103,48 @@ describe "ExtendedDocument", "protected flag" do lambda { WithBoth.new }.should raise_error end end + +describe "ExtendedDocument", "from database" do + class WithProtected < CouchRest::ExtendedDocument + use_database TEST_SERVER.default_database + property :name + property :admin, :default => false, :protected => true + view_by :name + end + + before(:each) do + @user = WithProtected.new + @user.name = "will" + @user.admin = true + @user.save! + end + + def verify_attrs(user) + user.name.should == "will" + user.admin.should == true + end + + it "ExtendedDocument#get should not strip protected attributes" do + reloaded = WithProtected.get( @user.id ) + verify_attrs reloaded + end + + it "ExtendedDocument#get! should not strip protected attributes" do + reloaded = WithProtected.get!( @user.id ) + verify_attrs reloaded + end + + it "ExtendedDocument#all should not strip protected attributes" do + # all creates a CollectionProxy + docs = WithProtected.all(:key => @user.id) + docs.size.should == 1 + reloaded = docs.first + verify_attrs reloaded + end + + it "views should not strip protected attributes" do + docs = WithProtected.by_name(:startkey => "will", :endkey => "will") + reloaded = docs.first + verify_attrs reloaded + end +end