Skip to content
This repository has been archived by the owner on Apr 17, 2018. It is now read-only.

Commit

Permalink
Fixed problem with Resource#save failing with custom properties
Browse files Browse the repository at this point in the history
* Resource#save now tests the loaded value rather than the dumped
  value for validity.
* Updated variable names to differentiate between a loaded value
  and a dumped value.  A loaded value is what is set within the
  Resource after retrieving/loading/typecasting it.  A dumped
  value is what is stored in the datastore.
* Fixed Object type so if loaded value is nil, it will just store
  nil rather than sending an encoded String to the datastore
* Fixed Property#primitive? so that properties with a Text primitive
  can be tested for validity.
* Added specs for Object type

[#1118 state:resolved]
  • Loading branch information
dkubb committed Nov 10, 2009
1 parent 8611d3a commit e66c1bd
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 42 deletions.
2 changes: 2 additions & 0 deletions Manifest.txt
Expand Up @@ -86,6 +86,7 @@ spec/public/collection_spec.rb
spec/public/migrations_spec.rb
spec/public/model/relationship_spec.rb
spec/public/model_spec.rb
spec/public/property/object_spec.rb
spec/public/property_spec.rb
spec/public/resource_spec.rb
spec/public/sel_spec.rb
Expand All @@ -111,6 +112,7 @@ spec/semipublic/associations/relationship_spec.rb
spec/semipublic/associations_spec.rb
spec/semipublic/collection_spec.rb
spec/semipublic/property_spec.rb
spec/semipublic/query/conditions/operation_spec.rb
spec/semipublic/query/conditions_spec.rb
spec/semipublic/query/path_spec.rb
spec/semipublic/query_spec.rb
Expand Down
4 changes: 2 additions & 2 deletions dm-core.gemspec
Expand Up @@ -6,11 +6,11 @@ Gem::Specification.new do |s|

s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version=
s.authors = ["Dan Kubb"]
s.date = %q{2009-10-27}
s.date = %q{2009-11-09}
s.description = %q{Faster, Better, Simpler.}
s.email = ["dan.kubb@gmail.com"]
s.extra_rdoc_files = ["History.txt", "Manifest.txt", "README.txt"]
s.files = [".autotest", ".gitignore", "CONTRIBUTING", "FAQ", "History.txt", "MIT-LICENSE", "Manifest.txt", "QUICKLINKS", "README.txt", "Rakefile", "SPECS", "TODO", "deps.rip", "dm-core.gemspec", "lib/dm-core.rb", "lib/dm-core/adapters.rb", "lib/dm-core/adapters/abstract_adapter.rb", "lib/dm-core/adapters/data_objects_adapter.rb", "lib/dm-core/adapters/in_memory_adapter.rb", "lib/dm-core/adapters/mysql_adapter.rb", "lib/dm-core/adapters/oracle_adapter.rb", "lib/dm-core/adapters/postgres_adapter.rb", "lib/dm-core/adapters/sqlite3_adapter.rb", "lib/dm-core/adapters/sqlserver_adapter.rb", "lib/dm-core/adapters/yaml_adapter.rb", "lib/dm-core/associations/many_to_many.rb", "lib/dm-core/associations/many_to_one.rb", "lib/dm-core/associations/one_to_many.rb", "lib/dm-core/associations/one_to_one.rb", "lib/dm-core/associations/relationship.rb", "lib/dm-core/collection.rb", "lib/dm-core/core_ext/enumerable.rb", "lib/dm-core/core_ext/kernel.rb", "lib/dm-core/core_ext/symbol.rb", "lib/dm-core/identity_map.rb", "lib/dm-core/migrations.rb", "lib/dm-core/model.rb", "lib/dm-core/model/descendant_set.rb", "lib/dm-core/model/hook.rb", "lib/dm-core/model/is.rb", "lib/dm-core/model/property.rb", "lib/dm-core/model/relationship.rb", "lib/dm-core/model/scope.rb", "lib/dm-core/property.rb", "lib/dm-core/property_set.rb", "lib/dm-core/query.rb", "lib/dm-core/query/conditions/comparison.rb", "lib/dm-core/query/conditions/operation.rb", "lib/dm-core/query/direction.rb", "lib/dm-core/query/operator.rb", "lib/dm-core/query/path.rb", "lib/dm-core/query/sort.rb", "lib/dm-core/repository.rb", "lib/dm-core/resource.rb", "lib/dm-core/spec/adapter_shared_spec.rb", "lib/dm-core/spec/data_objects_adapter_shared_spec.rb", "lib/dm-core/support/chainable.rb", "lib/dm-core/support/deprecate.rb", "lib/dm-core/support/equalizer.rb", "lib/dm-core/support/logger.rb", "lib/dm-core/support/naming_conventions.rb", "lib/dm-core/transaction.rb", "lib/dm-core/type.rb", "lib/dm-core/types/boolean.rb", "lib/dm-core/types/discriminator.rb", "lib/dm-core/types/object.rb", "lib/dm-core/types/paranoid_boolean.rb", "lib/dm-core/types/paranoid_datetime.rb", "lib/dm-core/types/serial.rb", "lib/dm-core/types/text.rb", "lib/dm-core/version.rb", "script/performance.rb", "script/profile.rb", "spec/lib/adapter_helpers.rb", "spec/lib/collection_helpers.rb", "spec/lib/counter_adapter.rb", "spec/lib/pending_helpers.rb", "spec/lib/rspec_immediate_feedback_formatter.rb", "spec/public/associations/many_to_many_spec.rb", "spec/public/associations/many_to_one_spec.rb", "spec/public/associations/many_to_one_with_boolean_cpk_spec.rb", "spec/public/associations/one_to_many_spec.rb", "spec/public/associations/one_to_one_spec.rb", "spec/public/associations/one_to_one_with_boolean_cpk_spec.rb", "spec/public/collection_spec.rb", "spec/public/migrations_spec.rb", "spec/public/model/relationship_spec.rb", "spec/public/model_spec.rb", "spec/public/property_spec.rb", "spec/public/resource_spec.rb", "spec/public/sel_spec.rb", "spec/public/setup_spec.rb", "spec/public/shared/association_collection_shared_spec.rb", "spec/public/shared/collection_finder_shared_spec.rb", "spec/public/shared/collection_shared_spec.rb", "spec/public/shared/finder_shared_spec.rb", "spec/public/shared/resource_shared_spec.rb", "spec/public/shared/sel_shared_spec.rb", "spec/public/transaction_spec.rb", "spec/public/types/discriminator_spec.rb", "spec/semipublic/adapters/abstract_adapter_spec.rb", "spec/semipublic/adapters/in_memory_adapter_spec.rb", "spec/semipublic/adapters/mysql_adapter_spec.rb", "spec/semipublic/adapters/oracle_adapter_spec.rb", "spec/semipublic/adapters/postgres_adapter_spec.rb", "spec/semipublic/adapters/sqlite3_adapter_spec.rb", "spec/semipublic/adapters/sqlserver_adapter_spec.rb", "spec/semipublic/adapters/yaml_adapter_spec.rb", "spec/semipublic/associations/many_to_one_spec.rb", "spec/semipublic/associations/relationship_spec.rb", "spec/semipublic/associations_spec.rb", "spec/semipublic/collection_spec.rb", "spec/semipublic/property_spec.rb", "spec/semipublic/query/conditions_spec.rb", "spec/semipublic/query/path_spec.rb", "spec/semipublic/query_spec.rb", "spec/semipublic/resource_spec.rb", "spec/semipublic/shared/condition_shared_spec.rb", "spec/semipublic/shared/resource_shared_spec.rb", "spec/spec.opts", "spec/spec_helper.rb", "tasks/ci.rb", "tasks/dm.rb", "tasks/doc.rb", "tasks/gemspec.rb", "tasks/hoe.rb", "tasks/install.rb"]
s.files = [".autotest", ".gitignore", "CONTRIBUTING", "FAQ", "History.txt", "MIT-LICENSE", "Manifest.txt", "QUICKLINKS", "README.txt", "Rakefile", "SPECS", "TODO", "deps.rip", "dm-core.gemspec", "lib/dm-core.rb", "lib/dm-core/adapters.rb", "lib/dm-core/adapters/abstract_adapter.rb", "lib/dm-core/adapters/data_objects_adapter.rb", "lib/dm-core/adapters/in_memory_adapter.rb", "lib/dm-core/adapters/mysql_adapter.rb", "lib/dm-core/adapters/oracle_adapter.rb", "lib/dm-core/adapters/postgres_adapter.rb", "lib/dm-core/adapters/sqlite3_adapter.rb", "lib/dm-core/adapters/sqlserver_adapter.rb", "lib/dm-core/adapters/yaml_adapter.rb", "lib/dm-core/associations/many_to_many.rb", "lib/dm-core/associations/many_to_one.rb", "lib/dm-core/associations/one_to_many.rb", "lib/dm-core/associations/one_to_one.rb", "lib/dm-core/associations/relationship.rb", "lib/dm-core/collection.rb", "lib/dm-core/core_ext/enumerable.rb", "lib/dm-core/core_ext/kernel.rb", "lib/dm-core/core_ext/symbol.rb", "lib/dm-core/identity_map.rb", "lib/dm-core/migrations.rb", "lib/dm-core/model.rb", "lib/dm-core/model/descendant_set.rb", "lib/dm-core/model/hook.rb", "lib/dm-core/model/is.rb", "lib/dm-core/model/property.rb", "lib/dm-core/model/relationship.rb", "lib/dm-core/model/scope.rb", "lib/dm-core/property.rb", "lib/dm-core/property_set.rb", "lib/dm-core/query.rb", "lib/dm-core/query/conditions/comparison.rb", "lib/dm-core/query/conditions/operation.rb", "lib/dm-core/query/direction.rb", "lib/dm-core/query/operator.rb", "lib/dm-core/query/path.rb", "lib/dm-core/query/sort.rb", "lib/dm-core/repository.rb", "lib/dm-core/resource.rb", "lib/dm-core/spec/adapter_shared_spec.rb", "lib/dm-core/spec/data_objects_adapter_shared_spec.rb", "lib/dm-core/support/chainable.rb", "lib/dm-core/support/deprecate.rb", "lib/dm-core/support/equalizer.rb", "lib/dm-core/support/logger.rb", "lib/dm-core/support/naming_conventions.rb", "lib/dm-core/transaction.rb", "lib/dm-core/type.rb", "lib/dm-core/types/boolean.rb", "lib/dm-core/types/discriminator.rb", "lib/dm-core/types/object.rb", "lib/dm-core/types/paranoid_boolean.rb", "lib/dm-core/types/paranoid_datetime.rb", "lib/dm-core/types/serial.rb", "lib/dm-core/types/text.rb", "lib/dm-core/version.rb", "script/performance.rb", "script/profile.rb", "spec/lib/adapter_helpers.rb", "spec/lib/collection_helpers.rb", "spec/lib/counter_adapter.rb", "spec/lib/pending_helpers.rb", "spec/lib/rspec_immediate_feedback_formatter.rb", "spec/public/associations/many_to_many_spec.rb", "spec/public/associations/many_to_one_spec.rb", "spec/public/associations/many_to_one_with_boolean_cpk_spec.rb", "spec/public/associations/one_to_many_spec.rb", "spec/public/associations/one_to_one_spec.rb", "spec/public/associations/one_to_one_with_boolean_cpk_spec.rb", "spec/public/collection_spec.rb", "spec/public/migrations_spec.rb", "spec/public/model/relationship_spec.rb", "spec/public/model_spec.rb", "spec/public/property/object_spec.rb", "spec/public/property_spec.rb", "spec/public/resource_spec.rb", "spec/public/sel_spec.rb", "spec/public/setup_spec.rb", "spec/public/shared/association_collection_shared_spec.rb", "spec/public/shared/collection_finder_shared_spec.rb", "spec/public/shared/collection_shared_spec.rb", "spec/public/shared/finder_shared_spec.rb", "spec/public/shared/resource_shared_spec.rb", "spec/public/shared/sel_shared_spec.rb", "spec/public/transaction_spec.rb", "spec/public/types/discriminator_spec.rb", "spec/semipublic/adapters/abstract_adapter_spec.rb", "spec/semipublic/adapters/in_memory_adapter_spec.rb", "spec/semipublic/adapters/mysql_adapter_spec.rb", "spec/semipublic/adapters/oracle_adapter_spec.rb", "spec/semipublic/adapters/postgres_adapter_spec.rb", "spec/semipublic/adapters/sqlite3_adapter_spec.rb", "spec/semipublic/adapters/sqlserver_adapter_spec.rb", "spec/semipublic/adapters/yaml_adapter_spec.rb", "spec/semipublic/associations/many_to_one_spec.rb", "spec/semipublic/associations/relationship_spec.rb", "spec/semipublic/associations_spec.rb", "spec/semipublic/collection_spec.rb", "spec/semipublic/property_spec.rb", "spec/semipublic/query/conditions/operation_spec.rb", "spec/semipublic/query/conditions_spec.rb", "spec/semipublic/query/path_spec.rb", "spec/semipublic/query_spec.rb", "spec/semipublic/resource_spec.rb", "spec/semipublic/shared/condition_shared_spec.rb", "spec/semipublic/shared/resource_shared_spec.rb", "spec/spec.opts", "spec/spec_helper.rb", "tasks/ci.rb", "tasks/dm.rb", "tasks/doc.rb", "tasks/gemspec.rb", "tasks/hoe.rb", "tasks/install.rb"]
s.homepage = %q{http://datamapper.org}
s.rdoc_options = ["--main", "README.txt"]
s.require_paths = ["lib"]
Expand Down
18 changes: 10 additions & 8 deletions lib/dm-core/property.rb
Expand Up @@ -685,33 +685,33 @@ def default?
# Returns given value unchanged for core types and
# uses +dump+ method of the property type for custom types.
#
# @param [Object] value
# @param [Object] loaded_value
# the value to be converted into a storeable (ie., primitive) value
#
# @return [Object]
# the primitive value to be stored in the repository for +val+
#
# @api semipublic
def value(value)
def value(loaded_value)
if custom?
type.dump(value, self)
type.dump(loaded_value, self)
else
value
loaded_value
end
end

# Test the value to see if it is a valid value for this Property
#
# @param [Object] value
# @param [Object] loaded_value
# the value to be tested
#
# @return [Boolean]
# true if the value is valid
#
# @api semipulic
def valid?(value, negated = false)
value = self.value(value)
primitive?(value) || (value.nil? && (nullable? || negated))
def valid?(loaded_value, negated = false)
dumped_value = self.value(loaded_value)
primitive?(dumped_value) || (dumped_value.nil? && (nullable? || negated))
end

# Returns a concise string representation of the property instance.
Expand All @@ -736,6 +736,8 @@ def inspect
def primitive?(value)
if primitive == TrueClass
value == true || value == false
elsif primitive == Types::Text
value.kind_of?(String)
else
value.kind_of?(primitive)
end
Expand Down
49 changes: 24 additions & 25 deletions lib/dm-core/query/conditions/comparison.rb
Expand Up @@ -112,7 +112,7 @@ class AbstractComparison

deprecate :property, :subject

equalize :slug, :subject, :value
equalize :slug, :subject, :dumped_value

# @api semipublic
attr_reader :parent
Expand All @@ -136,7 +136,9 @@ class AbstractComparison
# @return [Object]
#
# @api semipublic
attr_reader :value
attr_reader :dumped_value

alias value dumped_value

# The loaded/typecast value
#
Expand Down Expand Up @@ -252,7 +254,7 @@ def property?
# @api semipublic
def inspect
"#<#{self.class} @subject=#{@subject.inspect} " \
"@value=#{@value.inspect} @loaded_value=#{@loaded_value.inspect}>"
"@dumped_value=#{@dumped_value.inspect} @loaded_value=#{@loaded_value.inspect}>"
end

# Returns a string version of this Comparison object
Expand All @@ -265,7 +267,7 @@ def inspect
#
# @api semipublic
def to_s
"#{@subject.name} #{comparator_string} #{@value}"
"#{subject.name} #{comparator_string} #{dumped_value}"
end

# @api private
Expand All @@ -288,7 +290,7 @@ def negated?
def initialize(subject, value)
@subject = subject
@loaded_value = typecast_value(value)
@value = dumped_value(@loaded_value)
@dumped_value = dump_value
end

# Typecasts the given +val+ using subject#typecast
Expand All @@ -313,25 +315,22 @@ def typecast_value(val)
end
end

# Dumps the given +val+ using subject#value
# Dumps the given loaded_value using subject#value
#
# This converts property values to the primitive as stored in the
# repository.
#
# @param [Object] val
# The object to attempt to typecast.
#
# @return [Object]
# The raw (dumped) object.
#
# @see Property#value
#
# @api private
def dumped_value(val)
def dump_value
if subject.respond_to?(:value)
subject.value(val)
subject.value(loaded_value)
else
val
loaded_value
end
end

Expand Down Expand Up @@ -423,8 +422,8 @@ def expected_value(val = @loaded_value)
# @return [Boolean] true if the value is valid
#
# @api semipublic
def valid_for_subject?(value)
subject.valid?(value, negated?)
def valid_for_subject?(loaded_value)
subject.valid?(loaded_value, negated?)
end

# @api private
Expand Down Expand Up @@ -571,18 +570,18 @@ def typecast_value(val)
#
# @return [Array<Object>]
#
# @see AbtractComparison#dumped_value
# @see AbtractComparison#dump_value
#
# @api private
def dumped_value(val)
if subject.respond_to?(:value) && val.kind_of?(Range) && !subject.custom?
val
elsif subject.respond_to?(:value) && val.respond_to?(:map)
val = val.map { |el| subject.value(el) }
val.uniq!
val
def dump_value
if subject.respond_to?(:value) && loaded_value.kind_of?(Range) && !subject.custom?
loaded_value
elsif subject.respond_to?(:value) && loaded_value.respond_to?(:map)
dumped_value = loaded_value.map { |value| subject.value(value) }
dumped_value.uniq!
dumped_value
else
val
loaded_value
end
end

Expand Down Expand Up @@ -620,7 +619,7 @@ def matches?(record)
#
# @api semipublic
def valid?
value.kind_of?(Regexp)
loaded_value.kind_of?(Regexp)
end

private
Expand Down Expand Up @@ -677,7 +676,7 @@ def matches?(record)
#
# @api semipublic
def expected_value
Regexp.new(@value.to_s.gsub('%', '.*').gsub('_', '.'))
Regexp.new(loaded_value.to_s.gsub('%', '.*').gsub('_', '.'))
end

# @return [String]
Expand Down
10 changes: 4 additions & 6 deletions lib/dm-core/resource.rb
Expand Up @@ -847,23 +847,21 @@ def _create
#
# @api private
def _update
dirty_attributes = self.dirty_attributes

if dirty_attributes.empty?
if original_attributes.empty?
true
elsif dirty_attributes.any? { |property, value| !property.valid?(value) }
elsif original_attributes.any? { |property, _| !property.valid?(property.get!(self)) }
false
else
# remove from the identity map
identity_map.delete(key)

repository.update(dirty_attributes, collection_for_self)

original_attributes.clear

# remove the cached key in case it is updated
remove_instance_variable(:@_key)

original_attributes.clear

identity_map[key] = self

true
Expand Down
2 changes: 1 addition & 1 deletion lib/dm-core/types/object.rb
Expand Up @@ -10,7 +10,7 @@ def self.typecast(value, property)

# @api private
def self.dump(value, property)
[ Marshal.dump(value) ].pack('m')
[ Marshal.dump(value) ].pack('m') unless value.nil?
end

# @api private
Expand Down
100 changes: 100 additions & 0 deletions spec/public/property/object_spec.rb
@@ -0,0 +1,100 @@
require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'spec_helper'))

describe DataMapper::Property, 'Object type' do
before :all do
module ::Blog
class Article
include DataMapper::Resource

property :id, Serial
property :title, String
property :meta, Object, :nullable => false
end
end

@model = Blog::Article
@property = @model.properties[:meta]
end

subject { @property }

it { should respond_to(:typecast) }

describe '#typecast' do
before do
@value = { 'lang' => 'en_CA' }
end

subject { @property.typecast(@value) }

it { should equal(@value) }
end

it { should respond_to(:value) }

describe '#value' do
describe 'with a value' do
subject { @property.value('lang' => 'en_CA') }

it { should == "BAh7BiIJbGFuZyIKZW5fQ0E=\n" }
end

describe 'with nil' do
subject { @property.value(nil) }

it { should be_nil }
end
end

it { should respond_to(:valid?) }

describe '#valid?' do
describe 'with a valid primitive' do
subject { @property.valid?('lang' => 'en_CA') }

it { should be_true }
end

describe 'with nil and property is nullable' do
before do
@property = @model.property(:meta, Object, :nullable => true)
end

subject { @property.valid?(nil) }

it { should be_true }
end

describe 'with nil and property is not nullable' do
subject { @property.valid?(nil) }

it { should be_false }
end

describe 'with nil and property is not nullable, but validity is negated' do
subject { @property.valid?(nil, true) }

it { should be_true }
end
end

describe 'persistable' do
supported_by :all do
before :all do
@do_adapter = defined?(DataMapper::Adapters::DataObjectsAdapter) && @adapter.kind_of?(DataMapper::Adapters::DataObjectsAdapter)
end

before :all do
@resource = @model.create(:title => 'Test', :meta => { 'lang' => 'en_CA' })
end

subject { @resource.reload.meta }

it 'should load the correct value' do
pending_if 'Fix adapters to use different serialization methods', !@do_adapter do
should == { 'lang' => 'en_CA' }
end
end
end
end
end

0 comments on commit e66c1bd

Please sign in to comment.