Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Save bug fix #37

Merged
merged 2 commits into from

2 participants

@raphaelcm

Makes a couple tweaks so that tests can run "out of the box" on vanilla systems.

Also fixes two bugs:

Bug 1

Given the following setup:

class ActiveRecordUser < ActiveRecord::Base
  include Tenacity

  belongs_to :active_record_organization, :autosave => true
end

class ActiveRecordOrganization < ActiveRecord::Base
  include Tenacity

  has_many :active_record_users
end

class MongoidCampusHub
  include Mongoid::Document
  include Tenacity

  t_belongs_to :active_record_organization

end

This scenario throws an error:

org = ActiveRecordOrganization.create
campus_hub = MongoidCampusHub.create
campus_hub.active_record_organization = org
campus_hub.save!
user = ActiveRecordUser.new
user.active_record_organization = campus_hub.active_record_organization
user.save

Here's the error:

ArgumentError: wrong number of arguments (1 for 0)
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/associations/association_proxy.rb:225:in `save'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/associations/association_proxy.rb:225:in `send'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/associations/association_proxy.rb:225:in `method_missing'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/autosave_association.rb:360:in `save_belongs_to_association'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/autosave_association.rb:172:in `autosave_associated_records_for_organization'
    from /var/bundler/turtle/ruby/1.8/gems/activesupport-3.0.7/lib/active_support/callbacks.rb:420:in `_run_save_callbacks'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/callbacks.rb:273:in `create_or_update'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/persistence.rb:39:in `save'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/validations.rb:43:in `save'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/attribute_methods/dirty.rb:21:in `save'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/transactions.rb:240:in `save'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/transactions.rb:292:in `with_transaction_returning_status'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/connection_adapters/abstract/database_statements.rb:139:in `transaction'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/transactions.rb:207:in `transaction'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/transactions.rb:290:in `with_transaction_returning_status'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/transactions.rb:240:in `save'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/transactions.rb:251:in `rollback_active_record_state!'
    from /var/bundler/turtle/ruby/1.8/gems/activerecord-3.0.7/lib/active_record/transactions.rb:239:in `save'

It's caused because the save method in associate_proxy.rb doesn't accept any arguments, such as :validate => false, which gets automatically sent by ActiveRecord.

Bug 2

If an object is unchanged, save returns false. This causes problems in typical controller update methods:

def update
  @my_obj.update_attributes(params[:my_obj_attributes])

  if @my_obj.save
    render :index
  else
    render :edit
  end
end

If the user visits the edit form and clicks 'update' without making any changes, the controller will render the edit form again, instead of going to the :index action, as it should.

@jwood
Owner

This looks great Raphael, thanks! I'll get this merged in soon.

@jwood jwood merged commit 56e5a6b into jwood:master
@raphaelcm
@jwood
Owner

Fixed in version 0.5.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 23, 2012
  1. @raphaelcm
  2. @raphaelcm

    Fix a couple bugs.

    raphaelcm authored
This page is out of date. Refresh to see the latest.
View
4 lib/tenacity/associate_proxy.rb
@@ -23,11 +23,11 @@ def inspect
@target.inspect
end
- def save
+ def save(*args)
if @association.readonly?
raise ReadOnlyError
else
- @target._t_save_if_dirty
+ @target._t_save_if_dirty(*args)
end
end
View
4 lib/tenacity/orm_ext/activerecord.rb
@@ -112,8 +112,8 @@ def _t_reload
self
end
- def _t_save_if_dirty
- changed? ? save : false
+ def _t_save_if_dirty(*args)
+ changed? ? save(*args) : true
end
end
View
4 lib/tenacity/orm_ext/couchrest.rb
@@ -145,8 +145,8 @@ def _t_reload
self
end
- def _t_save_if_dirty
- save
+ def _t_save_if_dirty(*args)
+ save(*args)
end
end
View
4 lib/tenacity/orm_ext/datamapper.rb
@@ -138,8 +138,8 @@ def _t_reload
self.class._t_find(self.id)
end
- def _t_save_if_dirty
- dirty? ? save : false
+ def _t_save_if_dirty(*args)
+ dirty? ? save(*args) : true
end
private
View
4 lib/tenacity/orm_ext/mongo_mapper.rb
@@ -111,8 +111,8 @@ def _t_reload
self
end
- def _t_save_if_dirty
- changed? ? save : false
+ def _t_save_if_dirty(*args)
+ changed? ? save(*args) : true
end
end
View
4 lib/tenacity/orm_ext/mongoid.rb
@@ -113,8 +113,8 @@ def _t_reload
self
end
- def _t_save_if_dirty
- changed? ? save : false
+ def _t_save_if_dirty(*args)
+ changed? ? save(*args) : true
end
end
View
4 lib/tenacity/orm_ext/ripple.rb
@@ -139,8 +139,8 @@ def _t_reload
self
end
- def _t_save_if_dirty
- changed? ? save : false
+ def _t_save_if_dirty(*args)
+ changed? ? save(*args) : true
end
def save
View
4 lib/tenacity/orm_ext/sequel.rb
@@ -147,8 +147,8 @@ def _t_reload
self
end
- def _t_save_if_dirty
- !changed_columns.empty? ? save : false
+ def _t_save_if_dirty(*args)
+ !changed_columns.empty? ? save(*args) : true
end
end
View
4 lib/tenacity/orm_ext/toystore.rb
@@ -109,8 +109,8 @@ def _t_reload
self
end
- def _t_save_if_dirty
- changed? ? save : false
+ def _t_save_if_dirty(*args)
+ changed? ? save(*args) : true
end
end
end
View
4 tenacity.gemspec
@@ -27,14 +27,14 @@ Gem::Specification.new do |s|
# Relational DBs
s.add_development_dependency "sqlite3-ruby", "1.3.1"
- s.add_development_dependency "activerecord", "~> 3.0.0"
+ s.add_development_dependency "activerecord", "= 3.0.7"
s.add_development_dependency "datamapper", "1.0.2"
s.add_development_dependency "dm-sqlite-adapter", "1.0.2"
s.add_development_dependency "sequel", "3.19.0"
# MongoDB
s.add_development_dependency "mongo_mapper", "0.9.0"
- s.add_development_dependency "bson_ext", "1.3.1"
+ s.add_development_dependency "bson_ext", "1.6.2"
s.add_development_dependency "mongoid", "2.0.0"
# CouchDB
View
5 test/fixtures/active_record_organization.rb
@@ -0,0 +1,5 @@
+class ActiveRecordOrganization < ActiveRecord::Base
+ include Tenacity
+
+ has_many :active_record_users
+end
View
5 test/fixtures/active_record_user.rb
@@ -0,0 +1,5 @@
+class ActiveRecordUser < ActiveRecord::Base
+ include Tenacity
+
+ belongs_to :active_record_organization, :autosave => true
+end
View
8 test/fixtures/mongoid_campus_hub.rb
@@ -0,0 +1,8 @@
+class MongoidCampusHub
+ include Mongoid::Document
+ include Mongoid::Timestamps
+ include Tenacity
+
+ t_belongs_to :active_record_organization
+
+end
View
8 test/helpers/active_record_test_helper.rb
@@ -29,6 +29,14 @@
create_table :active_record_object_with_string_ids, :force => true, :id => false do |t|
t.string :id, :limit => 36, :primary => true
end
+
+ create_table :active_record_users, :force => true do |t|
+ t.integer :active_record_organization_id
+ end
+
+ create_table :active_record_organizations, :force => true do |t|
+
+ end
create_table :active_record_has_one_targets, :force => true do |t|
t.integer :active_record_object_id
View
19 test/orm_ext/activerecord_test.rb
@@ -98,9 +98,15 @@ class ActiveRecordTest < Test::Unit::TestCase
assert object._t_save_if_dirty
end
+ should "return true for save if valid object is not dirty" do
+ object = ActiveRecordObject.create
+ assert object.save
+ end
+
should "not save the object if it is not dirty" do
object = ActiveRecordObject.create
- assert !object._t_save_if_dirty
+ ActiveRecordObject.any_instance.stubs(:save).returns(false)
+ assert object._t_save_if_dirty
end
should "be able to successfully determine the id type" do
@@ -110,6 +116,17 @@ class ActiveRecordTest < Test::Unit::TestCase
class ActiveRecordObjectWithNoTable < ActiveRecord::Base; include Tenacity; end
assert_equal Integer, ActiveRecordObjectWithNoTable._t_id_type
end
+
+ should "successfully save if belongs_to another AR object which is assigned from a mongoid object" do
+ org = ActiveRecordOrganization.create
+ campus_hub = MongoidCampusHub.create
+ campus_hub.active_record_organization = org
+ campus_hub.save!
+ user = ActiveRecordUser.new
+ user.active_record_organization = campus_hub.active_record_organization
+ assert user.save
+ assert user.active_record_organization.save(:validate => false)
+ end
end
private
View
8 test/orm_ext/datamapper_test.rb
@@ -89,9 +89,15 @@ class DataMapperTest < Test::Unit::TestCase
assert object._t_save_if_dirty
end
+ should "return true for save if valid object is not dirty" do
+ object = DataMapperObject.create
+ assert object.save
+ end
+
should "not save the object if it is not dirty" do
object = DataMapperObject.create
- assert !object._t_save_if_dirty
+ DataMapperObject.any_instance.stubs(:save).returns(false)
+ assert object._t_save_if_dirty
end
should "be able to successfully determine the id type" do
View
8 test/orm_ext/mongo_mapper_test.rb
@@ -99,9 +99,15 @@ class MongoMapperTest < Test::Unit::TestCase
assert object._t_save_if_dirty
end
+ should "return true for save if valid object is not dirty" do
+ object = MongoMapperObject.create
+ assert object.save
+ end
+
should "not save the object if it is not dirty" do
object = MongoMapperObject.create
- assert !object._t_save_if_dirty
+ MongoMapperObject.any_instance.stubs(:save).returns(false)
+ assert object._t_save_if_dirty
end
end
View
8 test/orm_ext/mongoid_test.rb
@@ -100,9 +100,15 @@ class MongoidTest < Test::Unit::TestCase
assert object._t_save_if_dirty
end
+ should "return true for save if valid object is not dirty" do
+ object = MongoidObject.create
+ assert object.save
+ end
+
should "not save the object if it is not dirty" do
object = MongoidObject.create
- assert !object._t_save_if_dirty
+ MongoidObject.any_instance.stubs(:save).returns(false)
+ assert object._t_save_if_dirty
end
end
View
8 test/orm_ext/ripple_test.rb
@@ -139,9 +139,15 @@ class RippleTest < Test::Unit::TestCase
assert object._t_save_if_dirty
end
+ should "return true for save if valid object is not dirty" do
+ object = RippleObject.create
+ assert object.save
+ end
+
should "not save the object if it is not dirty" do
object = RippleObject.create
- assert !object._t_save_if_dirty
+ RippleObject.any_instance.stubs(:save).returns(false)
+ assert object._t_save_if_dirty
end
end
View
8 test/orm_ext/sequel_test.rb
@@ -99,9 +99,15 @@ class SequelTest < Test::Unit::TestCase
assert object._t_save_if_dirty
end
+ should "return true for save if valid object is not dirty" do
+ object = SequelObject.create
+ assert object.save
+ end
+
should "not save the object if it is not dirty" do
object = SequelObject.create
- assert !object._t_save_if_dirty
+ SequelObject.any_instance.stubs(:save).returns(false)
+ assert object._t_save_if_dirty
end
should "be able to successfully determine the id type" do
View
8 test/orm_ext/toystore_test.rb
@@ -101,9 +101,15 @@ class ToystoreTest < Test::Unit::TestCase
assert object._t_save_if_dirty
end
+ should "return true for save if valid object is not dirty" do
+ object = ToystoreObject.create
+ assert object.save
+ end
+
should "not save the object if it is not dirty" do
object = ToystoreObject.create
- assert !object._t_save_if_dirty
+ ToystoreObject.any_instance.stubs(:save).returns(false)
+ assert object._t_save_if_dirty
end
end
View
2  test/test_helper.rb
@@ -35,7 +35,7 @@ def setup_fixtures
filename =~ /.*\/(.*)\.rb/
clazz = Kernel.const_get($1.camelcase)
if clazz.respond_to?(:delete_all)
- clazz.delete_all
+ clazz.delete_all rescue true
elsif clazz.respond_to?(:db)
clazz.db["delete from #{clazz.table_name}"].delete
elsif clazz.respond_to?(:destroy!)
Something went wrong with that request. Please try again.