has_many :dependent => :destroy deleting associations on update_attributes

[man2d]

Here's the code:

class Operation < ActiveRecord::Base
  belongs_to :service
  has_many :errors, :dependent => :destroy

  attr_accessible :name, :disabled, :timeout
  validates_uniqueness_of :name, :scope => :service_id


  def to_s
    self.name
  end
end

class Error < ActiveRecord::Base
  belongs_to :operation
  has_many :errors, :through => :operation # here's the funny mistake 
 end

So when I call @operation.update_attributes it destroys all errors associated with operation. The question is - why?!

[man2d]

Rails version: 3.2.6

[pftg]

@man2d may you check this bug for Rails 4.0?

[pftg]

By the way you may use this template

[acapilleri]

@man2d please could you provide:

• The @operation attributes before update
• The params that you pass to @operation.update_attributes

thanks in advance

[harssh]

Yes I can confirm this for 3.2.16 here's what I get on console

Loading development environment (Rails 3.2.16)
1.9.3-p392 :001 > o = Operation.new
 => #<Operation id: nil, name: nil, disabled: nil, created_at: nil, updated_at: nil> 
1.9.3-p392 :002 > o.name = "test"
 => "test" 
1.9.3-p392 :003 > o.save
   (0.1ms)  begin transaction
  SQL (19.0ms)  INSERT INTO "operations" ("created_at", "disabled", "name", "updated_at") VALUES (?, ?, ?, ?)  [["created_at", Mon, 09 Dec 2013 05:33:56 UTC +00:00], ["disabled", nil], ["name", "test"], ["updated_at", Mon, 09 Dec 2013 05:33:56 UTC +00:00]]
   (214.8ms)  commit transaction
 => true 
1.9.3-p392 :004 > e = Error.new 
 => #<Error id: nil, name: nil, created_at: nil, updated_at: nil> 
1.9.3-p392 :005 > e.name = "testerror"
 => "testerror" 
1.9.3-p392 :006 > e.save
   (0.1ms)  begin transaction
  SQL (0.5ms)  INSERT INTO "errors" ("created_at", "name", "updated_at") VALUES (?, ?, ?)  [["created_at", Mon, 09 Dec 2013 05:34:15 UTC +00:00], ["name", "testerror"], ["updated_at", Mon, 09 Dec 2013 05:34:15 UTC +00:00]]
   (236.1ms)  commit transaction
 => true 
1.9.3-p392 :007 > o
 => #<Operation id: 1, name: "test", disabled: nil, created_at: "2013-12-09 05:33:56", updated_at: "2013-12-09 05:33:56"> 
1.9.3-p392 :008 > o.errors
 => [] 
1.9.3-p392 :009 > o.errors << e
   (0.1ms)  begin transaction
   (0.3ms)  UPDATE "errors" SET "updated_at" = '2013-12-09 05:34:28.336022' WHERE "errors"."id" = 1
   (222.6ms)  commit transaction
 => [#<Error id: 1, name: "testerror", created_at: "2013-12-09 05:34:15", updated_at: "2013-12-09 05:34:28">] 
1.9.3-p392 :010 > o.errors
 => [#<Error id: 1, name: "testerror", created_at: "2013-12-09 05:34:15", updated_at: "2013-12-09 05:34:28">] 
1.9.3-p392 :011 > o.save
   (0.1ms)  begin transaction
  SQL (0.3ms)  DELETE FROM "errors" WHERE "errors"."id" = ?  [["id", 1]]
   (204.4ms)  commit transaction
 => true 

If I call o.save again it destroys error record

[harssh]

And yes I can confirm this for Rails 4 too

Loading development environment (Rails 4.0.0)
1.9.3-p392 :001 > o = Operation.new 
 => #<Operation id: nil, name: nil, disabled: nil, created_at: nil, updated_at: nil> 
1.9.3-p392 :002 > o.name = "rail4 test"
 => "rail4 test" 
1.9.3-p392 :003 > o.save
   (0.1ms)  begin transaction
In Rails 4.1 delete_all on associations would not fire callbacks. It means if the :dependent option is :destroy then the associated records would be deleted without loading and invoking callbacks.
  SQL (56.7ms)  INSERT INTO "operations" ("created_at", "name", "updated_at") VALUES (?, ?, ?)  [["created_at", Mon, 09 Dec 2013 06:12:07 UTC +00:00], ["name", "rail4 test"], ["updated_at", Mon, 09 Dec 2013 06:12:07 UTC +00:00]]
   (329.1ms)  commit transaction
 => true 
1.9.3-p392 :004 > e = Error.new 
 => #<Error id: nil, name: nil, created_at: nil, updated_at: nil, operation_id: nil> 
1.9.3-p392 :005 > e.name = "rails4 error"
 => "rails4 error" 
1.9.3-p392 :006 > e.save
   (0.1ms)  begin transaction
  SQL (0.5ms)  INSERT INTO "errors" ("created_at", "name", "updated_at") VALUES (?, ?, ?)  [["created_at", Mon, 09 Dec 2013 06:13:00 UTC +00:00], ["name", "rails4 error"], ["updated_at", Mon, 09 Dec 2013 06:13:00 UTC +00:00]]
   (285.6ms)  commit transaction
 => true 
1.9.3-p392 :007 > o.errors << e
   (0.1ms)  begin transaction
  SQL (0.5ms)  UPDATE "errors" SET "operation_id" = ?, "updated_at" = ? WHERE "errors"."id" = 1  [["operation_id", 1], ["updated_at", Mon, 09 Dec 2013 06:13:08 UTC +00:00]]
   (218.0ms)  commit transaction
 => #<ActiveRecord::Associations::CollectionProxy [#<Error id: 1, name: "rails4 error", created_at: "2013-12-09 06:13:00", updated_at: "2013-12-09 06:13:08", operation_id: 1>]> 
1.9.3-p392 :008 > o.save
   (0.1ms)  begin transaction
In Rails 4.1 delete_all on associations would not fire callbacks. It means if the :dependent option is :destroy then the associated records would be deleted without loading and invoking callbacks.
  SQL (0.3ms)  DELETE FROM "errors" WHERE "errors"."id" = ?  [["id", 1]]
   (205.9ms)  commit transaction
 => true 

Please let me know if I am doing something wrong here ..... but I get this on Rails 4 and Rails 3 too..

[abatay]

Ok

[man2d]

Is there any ideas why this is happening?

[laurocaetano]

Hi @man2d.
I couldn't reproduce this issue, could you please provide a script to reproduce this (targeting 4.0.2 or master) using this template as @pftg said?

[man2d]

I just tested it with activerecord-4.0.0, it raises an exception:
ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection: Cannot modify association 'Error#errors' because the source reflection class 'Error' is associated to 'Operation' via :has_many.

Only reproducable in rails 3:

gem 'activerecord', '3.2.6'
require 'active_record'
require 'minitest/autorun'
require 'logger'


ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :operations do |t|
    t.string :name
  end

  create_table :errors do |t|
    t.integer :operation_id
  end
end

class Operation < ActiveRecord::Base
  has_many :errors, :dependent => :destroy
end

class Error < ActiveRecord::Base
  belongs_to :operation
  has_many :errors, :through => :operation
end

class BugTest < MiniTest::Unit::TestCase
  def test_association_stuff
    op = Operation.create!
    op.errors << Error.create!   
    assert_equal 1, op.errors.size
    op.name = "blabla"
    op.save
    assert_equal 0, op.errors.size

  end
end

[laurocaetano]

Hi @man2d.
The reason that I couldn't reproduce your issue is that I was using other names for the models.

When you save the model, it cleans all the errors (ActiveModel::Errors), you can check it here. So the problem here is that your model Operation also has errors

Instead of:

class Operation < ActiveRecord::Base
  has_many :errors
end

You can fix this using other name, like this:

class Operation < ActiveRecord::Base
  has_many :operation_errors, class_name: 'Error'
end

[rafaelfranca]

We already check if an attribute is dangerous. I think we should check if an association is dangerous too. This is the only way to avoid issues like this.

[harssh]

Got it so I had the same issue and changing :errors to :operation_errors does solve the problem ......

[acapilleri]

Hi guys

the following test is green, for active record edge and 3.2.6. There is no bug here.

gem 'activerecord', '3.2.6'
require 'active_record'
require 'minitest/autorun'
require 'logger'


ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :operations do |t|
    t.string :name
  end

  create_table :error_operations do |t|
    t.integer :operation_id
  end
end

class Operation < ActiveRecord::Base
  has_many :error_operations, :dependent => :destroy
end

class ErrorOperation < ActiveRecord::Base
  belongs_to :operation
  has_many :error_operations, :through => :operation
end

class BugTest < MiniTest::Unit::TestCase
  def test_association_stuff
    op = Operation.create!
    op.error_operations << ErrorOperation.create!
    assert_equal 1, op.error_operations.size
    op.name = "blabla"
    op.save
    assert_equal 1, op.error_operations.size
  end
end

While

gem 'activerecord'#, '3.2.6'
require 'active_record'
require 'minitest/autorun'
require 'logger'


ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :operations do |t|
    t.string :name
  end

  create_table :errors do |t|
    t.integer :operation_id
  end
end

class Operation < ActiveRecord::Base
  has_many :errors, :dependent => :destroy
end

class Error < ActiveRecord::Base
  belongs_to :operation
  has_many :errors, :through => :operation
end

class BugTest < MiniTest::Unit::TestCase
  def test_association_stuff
    op = Operation.create!
    op.errors << Error.create!
    assert_equal 1, op.errors.size
    op.name = "blabla"
    op.save
    assert_equal 0, op.errors.size

  end
end

=>

ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection: Cannot modify association 'Error#errors' because the source reflection class 'Error' is associated to 'Operation' via :has_many.
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/associations/through_association.rb:76:in `ensure_mutable'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/associations/through_association.rb:40:in `construct_join_attributes'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/associations/has_many_through_association.rb:139:in `delete_records'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/associations/collection_association.rb:232:in `delete'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/associations/collection_association.rb:160:in `delete_all'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/associations/collection_proxy.rb:422:in `delete_all'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/associations/collection_proxy.rb:956:in `clear'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activemodel-4.0.1/lib/active_model/validations.rb:313:in `valid?'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/validations.rb:70:in `valid?'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/validations.rb:77:in `perform_validations'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/validations.rb:57:in `save!'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/attribute_methods/dirty.rb:41:in `save!'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/transactions.rb:275:in `block in save!'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/transactions.rb:326:in `block in with_transaction_returning_status'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/connection_adapters/abstract/database_statements.rb:202:in `block in transaction'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/connection_adapters/abstract/database_statements.rb:210:in `within_new_transaction'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/connection_adapters/abstract/database_statements.rb:202:in `transaction'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/transactions.rb:209:in `transaction'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/transactions.rb:323:in `with_transaction_returning_status'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/transactions.rb:275:in `save!'
    /Users/resca/.rvm/gems/ruby-2.0.0-p247/gems/activerecord-4.0.1/lib/active_record/validations.rb:41:in `create!'
    erros.rb:32:in `test_association_stuff'

IMHO it's very ugly circular association with bad names, I think that it can be closed