Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixing issues with dirty tracking on nested models and related improv…

…ements
  • Loading branch information...
commit 2cc119b3b3f9dc5d97ef82612f2b28083b92d624 1 parent 8c4505d
@samlown samlown authored
View
2  VERSION
@@ -1 +1 @@
-1.1.0.beta3
+1.1.0.beta4
View
2  couchrest_model.gemspec
@@ -25,7 +25,7 @@ Gem::Specification.new do |s|
s.add_dependency(%q<couchrest>, "1.1.0.pre2")
s.add_dependency(%q<mime-types>, "~> 1.15")
- s.add_dependency(%q<activemodel>, "~> 3.0.0")
+ s.add_dependency(%q<activemodel>, "~> 3.0.5")
s.add_dependency(%q<tzinfo>, "~> 0.3.22")
s.add_dependency(%q<railties>, "~> 3.0.0")
s.add_development_dependency(%q<rspec>, ">= 2.0.0")
View
6 history.txt
@@ -1,4 +1,4 @@
-== 1.1.0.beta3
+== 1.1.0.beta4
* Major changes:
* Fast Dirty Tracking! Many thanks to @sobakasu (Andrew Williams)
@@ -10,6 +10,10 @@
* Added "auto_update_design_doc" configuration option.
* Using #descending on View object will automatically swap startkey with endkey.
+== 1.1.0.beta3
+
+* Removed
+
== 1.1.0.beta2
* Minor enhancements:
View
10 lib/couchrest/model/base.rb
@@ -18,8 +18,8 @@ class Base < Document
include CouchRest::Model::Associations
include CouchRest::Model::Validations
include CouchRest::Model::Designs
- include CouchRest::Model::Dirty
include CouchRest::Model::CastedBy
+ include CouchRest::Model::Dirty
def self.subclasses
@subclasses ||= []
@@ -74,14 +74,6 @@ def self.method_missing(m, *args, &block)
super
end
- ### instance methods
-
- # Checks if we're the top document
- # (overrides base_doc? in casted_by.rb)
- def base_doc?
- !@casted_by
- end
-
## Compatibility with ActiveSupport and older frameworks
# Hack so that CouchRest::Document, which descends from Hash,
View
49 lib/couchrest/model/casted_array.rb
@@ -5,25 +5,36 @@
module CouchRest::Model
class CastedArray < Array
+ include CouchRest::Model::CastedBy
include CouchRest::Model::Dirty
- attr_accessor :casted_by
- attr_accessor :property
+ attr_accessor :casted_by_property
- def initialize(array, property)
- self.property = property
+ def initialize(array, property, parent = nil)
+ self.casted_by_property = property
+ self.casted_by = parent unless parent.nil?
super(array)
end
-
+
+ # Adding new entries
+
def << obj
- couchrest_parent_will_change! if use_dirty?
super(instantiate_and_cast(obj))
end
-
+
def push(obj)
- couchrest_parent_will_change! if use_dirty?
super(instantiate_and_cast(obj))
end
+ def unshift(obj)
+ super(instantiate_and_cast(obj))
+ end
+
+ def []= index, obj
+ value = instantiate_and_cast(obj, false)
+ couchrest_parent_will_change! if use_dirty? && value != self[index]
+ super(index, value)
+ end
+
def pop
couchrest_parent_will_change! if use_dirty? && self.length > 0
super
@@ -34,17 +45,6 @@ def shift
super
end
- def unshift(obj)
- couchrest_parent_will_change! if use_dirty?
- super(instantiate_and_cast(obj))
- end
-
- def []= index, obj
- value = instantiate_and_cast(obj)
- couchrest_parent_will_change! if use_dirty? && value != self[index]
- super(index, value)
- end
-
def clear
couchrest_parent_will_change! if use_dirty? && self.length > 0
super
@@ -52,11 +52,14 @@ def clear
protected
- def instantiate_and_cast(obj)
- if self.casted_by && self.property && obj.class != self.property.type_class
- self.property.cast_value(self.casted_by, obj)
+ def instantiate_and_cast(obj, change = true)
+ property = casted_by_property
+ couchrest_parent_will_change! if change && use_dirty?
+ if casted_by && property && obj.class != property.type_class
+ property.cast_value(casted_by, obj)
else
- obj.casted_by = self.casted_by if obj.respond_to?(:casted_by)
+ obj.casted_by = casted_by if obj.respond_to?(:casted_by)
+ obj.casted_by_property = casted_by_property if obj.respond_to?(:casted_by_property)
obj
end
end
View
14 lib/couchrest/model/casted_by.rb
@@ -4,6 +4,7 @@ module CastedBy
extend ActiveSupport::Concern
included do
self.send(:attr_accessor, :casted_by)
+ self.send(:attr_accessor, :casted_by_property)
end
# Gets a reference to the actual document in the DB
@@ -11,13 +12,22 @@ module CastedBy
# 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
+ casted_by ? casted_by.base_doc : nil
end
# Checks if we're the top document
def base_doc?
- false
+ !casted_by
end
+ # Provide the property this casted model instance has been
+ # used by. If it has not been set, search through the
+ # casted_by objects properties to try and find it.
+ #def casted_by_property
+ # return nil unless casted_by
+ # attrs = casted_by.attributes
+ # @casted_by_property ||= casted_by.properties.detect{ |k| attrs[k.to_s] === self }
+ #end
+
end
end
View
10 lib/couchrest/model/casted_hash.rb
@@ -3,8 +3,16 @@
module CouchRest::Model
class CastedHash < Hash
+ include CouchRest::Model::CastedBy
include CouchRest::Model::Dirty
- attr_accessor :casted_by
+ attr_accessor :casted_by_property
+
+ def self.[](hash, property, parent = nil)
+ obj = super(hash)
+ obj.casted_by_property = property
+ obj.casted_by = parent unless parent.nil?
+ obj
+ end
# needed for dirty
def attributes
View
21 lib/couchrest/model/casted_model.rb
@@ -1,6 +1,6 @@
module CouchRest::Model
module CastedModel
-
+
extend ActiveSupport::Concern
included do
@@ -10,8 +10,14 @@ module CastedModel
include CouchRest::Model::PropertyProtection
include CouchRest::Model::Associations
include CouchRest::Model::Validations
+ include CouchRest::Model::CastedBy
include CouchRest::Model::Dirty
- # attr_accessor :casted_by
+ class_eval do
+ # Override CastedBy's base_doc?
+ def base_doc?
+ false # Can never be base doc!
+ end
+ end
end
def initialize(keys = {})
@@ -21,7 +27,6 @@ def initialize(keys = {})
end
def []= key, value
- couchrest_attribute_will_change!(key) if self[key] != value
super(key.to_s, value)
end
@@ -29,17 +34,10 @@ 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?
- @casted_by.nil? ? true : @casted_by.new?
+ casted_by.nil? ? true : casted_by.new?
end
alias :new_record? :new?
@@ -68,4 +66,5 @@ def update_attributes_without_saving(hash)
alias :attributes= :update_attributes_without_saving
end
+
end
View
16 lib/couchrest/model/dirty.rb
@@ -10,7 +10,6 @@ module Model
# 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
included do
@@ -21,8 +20,8 @@ module Dirty
end
def use_dirty?
- bdoc = base_doc
- bdoc && !bdoc.disable_dirty
+ doc = base_doc
+ doc && !doc.disable_dirty
end
def couchrest_attribute_will_change!(attr)
@@ -32,16 +31,7 @@ def couchrest_attribute_will_change!(attr)
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
- attr = @casted_by.attributes
- @casted_by_attribute = attr.keys.detect { |k| attr[k] == self }
+ casted_by.couchrest_attribute_will_change!(casted_by_property.name) if casted_by_property
end
end
View
14 lib/couchrest/model/persistence.rb
@@ -30,7 +30,7 @@ def create!
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 !self.changed?
+ return true if !self.disable_dirty && !self.changed?
_run_update_callbacks do
_run_save_callbacks do
result = database.save_doc(self)
@@ -143,20 +143,14 @@ def create!(attributes = {})
# must be globally unique across all document types which share a
# database, so if you'd like to scope uniqueness to this class, you
# should use the class name as part of the unique id.
- def unique_id method = nil, &block
+ def unique_id(method = nil, &block)
if method
- define_method :get_unique_id do
- self.send(method)
- end
define_method :set_unique_id do
- self['_id'] ||= get_unique_id
+ self['_id'] ||= self.send(method)
end
elsif block
- define_method :get_unique_id do
- block.call(self)
- end
define_method :set_unique_id do
- uniqid = get_unique_id
+ uniqid = block.call(self)
raise ArgumentError, "unique_id block must not return nil" if uniqid.nil?
self['_id'] ||= uniqid
end
View
37 lib/couchrest/model/properties.rb
@@ -6,9 +6,9 @@ module Properties
included do
extlib_inheritable_accessor(:properties) unless self.respond_to?(:properties)
- extlib_inheritable_accessor(:property_by_name) unless self.respond_to?(:property_by_name)
+ extlib_inheritable_accessor(:properties_by_name) unless self.respond_to?(:properties_by_name)
self.properties ||= []
- self.property_by_name ||= {}
+ self.properties_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
@@ -20,6 +20,12 @@ def properties
self.class.properties
end
+ # Returns all the class's properties as a Hash where the key is the name
+ # of the property.
+ def properties_by_name
+ self.class.properties_by_name
+ end
+
# Returns the Class properties with their values
#
# ==== Returns
@@ -43,31 +49,10 @@ def read_attribute(property)
def write_attribute(property, value)
prop = find_property!(property)
value = prop.is_a?(String) ? value : prop.cast(self, value)
- attribute_will_change!(prop.name) if use_dirty? && self[prop.name] != value
+ couchrest_attribute_will_change!(prop.name) if use_dirty? && self[prop.name] != value
self[prop.name] = 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
- old_id = get_unique_id
- end
-
- ret = super(key, value)
-
- if check_id_change
- # if we have set an attribute that results in the _id changing (unique_id),
- # force changed? to return true so that the record can be saved
- new_id = get_unique_id
- changed_attributes["_id"] = new_id if old_id != new_id
- end
-
- ret
- end
-
# 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.
@@ -90,7 +75,7 @@ def set_attributes(hash)
protected
def find_property(property)
- property.is_a?(Property) ? property : self.class.property_by_name[property.to_s]
+ property.is_a?(Property) ? property : self.class.properties_by_name[property.to_s]
end
# The following methods should be accessable by the Model::Base Class, but not by anything else!
@@ -212,7 +197,7 @@ def define_property(name, options={}, &block)
validates_casted_model property.name
end
properties << property
- property_by_name[property.to_s] = property
+ properties_by_name[property.to_s] = property
property
end
View
10 lib/couchrest/model/property.rb
@@ -38,16 +38,13 @@ def cast(parent, value)
end
arr = value.collect { |data| cast_value(parent, data) }
# allow casted_by calls to be passed up chain by wrapping in CastedArray
- value = CastedArray.new(arr, self)
- value.casted_by = parent
+ CastedArray.new(arr, self, 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]
- value.casted_by = parent
+ CastedHash[value, self, parent]
elsif !value.nil?
- value = cast_value(parent, value)
+ cast_value(parent, value)
end
- value
end
# Cast an individual value, not an array
@@ -71,6 +68,7 @@ def default_value
def associate_casted_value_to_parent(parent, value)
value.casted_by = parent if value.respond_to?(:casted_by)
+ value.casted_by_property = self if value.respond_to?(:casted_by_property)
value
end
View
8 spec/couchrest/casted_model_spec.rb
@@ -79,6 +79,10 @@ class NotAHashButWithCastedModelMixin
@obj.name.should == 'Eric'
@obj.details['color'].should == 'orange'
end
+ it "should always return base_doc? as false" do
+ @obj.base_doc?.should be_false
+ end
+
end
describe "casted as an attribute, but without a value" do
@@ -132,6 +136,10 @@ class NotAHashButWithCastedModelMixin
@casted_obj.casted_by.should == @obj
end
+ it "should know which property casted it" do
+ @casted_obj.casted_by_property.should == @obj.properties.detect{|p| p.to_s == 'casted_attribute'}
+ end
+
it "should return nil for the 'no_value' attribute" do
@casted_obj.no_value.should be_nil
end
View
55 spec/couchrest/dirty_spec.rb
@@ -13,7 +13,7 @@ class WithCastedModelMixin < Hash
property :casted_attribute, WithCastedModelMixin
end
-class DummyModel < CouchRest::Model::Base
+class DirtyModel < CouchRest::Model::Base
use_database DB
property :casted_attribute, WithCastedModelMixin
@@ -25,6 +25,16 @@ class DummyModel < CouchRest::Model::Base
end
end
+class DirtyUniqueIdModel < CouchRest::Model::Base
+ use_database DB
+ attr_accessor :code
+ unique_id :code
+ property :title, String, :default => "Sample Title"
+ timestamps!
+
+ def code; self['_id'] || @code; end
+end
+
describe "Dirty" do
describe "changes" do
@@ -66,18 +76,21 @@ class DummyModel < CouchRest::Model::Base
end
it "should report no changes on a hash property with a default value" do
- @obj = DummyModel.new
+ @obj = DirtyModel.new
@obj.details.changed?.should be_false
end
-=begin
# match activerecord behaviour
- # not currently working - not too important
it "should report changes on a new object with attributes set" do
@card = Card.new(:first_name => "matt")
@card.changed?.should be_true
end
-=end
+
+ it "should report no changes on new object with 'unique_id' set" do
+ @obj = DirtyUniqueIdModel.new
+ @obj.changed?.should be_false
+ @obj.changes.should be_empty
+ end
it "should report no changes on objects fetched from the database" do
card_id = Card.create!(:first_name => "matt").id
@@ -156,15 +169,37 @@ class DummyModel < CouchRest::Model::Base
it "should report changes to casted models" do
@cat = Cat.create!(:name => "Felix", :favorite_toy => { :name => "Mouse" })
@cat = Cat.find(@cat.id)
- @cat.favorite_toy['name'] = 'Feather'
+ @cat.favorite_toy.name = 'Feather'
@cat.changed?.should be_true
end
+ it "should report changes to casted model in array" do
+ @obj = Cat.create!(:name => 'felix', :toys => [{:name => "Catnip"}])
+ @obj = Cat.get(@obj.id)
+ @obj.toys.first.name.should eql('Catnip')
+ @obj.toys.first.changed?.should be_false
+ @obj.changed?.should be_false
+ @obj.toys.first.name = "Super Catnip"
+ @obj.toys.first.changed?.should be_true
+ @obj.changed?.should be_true
+ end
+
+ it "should report changes to anonymous casted models in array" do
+ @obj = DirtyModel.create!(:sub_models => [{:title => "Sample"}])
+ @obj = DirtyModel.get(@obj.id)
+ @obj.sub_models.first.title.should eql("Sample")
+ @obj.sub_models.first.changed?.should be_false
+ @obj.changed?.should be_false
+ @obj.sub_models.first.title = "Another Sample"
+ @obj.sub_models.first.changed?.should be_true
+ @obj.changed?.should be_true
+ end
+
# casted arrays
def test_casted_array(change_expected)
- obj = DummyModel.create!
- obj = DummyModel.get(obj.id)
+ obj = DirtyModel.create!
+ obj = DirtyModel.get(obj.id)
array = obj.keywords
yield array, obj
if change_expected
@@ -249,8 +284,8 @@ def should_not_change_array
# Object, {} (casted hash)
def test_casted_hash(change_expected)
- obj = DummyModel.create!
- obj = DummyModel.get(obj.id)
+ obj = DirtyModel.create!
+ obj = DirtyModel.get(obj.id)
hash = obj.details
yield hash, obj
if change_expected
View
2  spec/couchrest/persistence_spec.rb
@@ -227,7 +227,7 @@
@templated['important-field'] = 'not-important'
@templated.save.should be_true
t = WithTemplateAndUniqueID.get('very-important')
- t.should == @templated
+ t.id.should == @templated.id
end
it "should raise an error when the id is taken" do
Please sign in to comment.
Something went wrong with that request. Please try again.