Avoid double callbacks in Rails >= 4.1

[pkmiec]

See https://github.com/rails/protected_attributes/issues/77

[rails-bot]

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @rafaelfranca (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[rafaelfranca]

Thank you for the pull request. Could you add a test case for the fix?

[pacoguzman]

I'm having another problem that it solved with this code too. To reproduce the problem I executed the following bug report template:

begin
  require 'bundler/inline'
rescue LoadError => e
  $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(true) do
  source 'https://rubygems.org'
  # Activate the gem you are reporting the issue against.
  gem 'activerecord', '4.1.14'
  gem 'sqlite3'
  gem 'protected_attributes', '1.1.3'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'

# Ensure backward compatibility with Minitest 4
Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test)

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :titles, force: true do |t|
  end

  create_table :contributors, force: true  do |t|
  end

  create_table :credits, force: true do |t|
    t.integer :title_id
    t.integer :contributor_id
  end
end

class Title < ActiveRecord::Base

end

class Contributor < ActiveRecord::Base
  has_many :credits

  accepts_nested_attributes_for :credits

  attr_accessible :credits_attributes
end

class Credit < ActiveRecord::Base
  belongs_to :title
  belongs_to :contributor

  accepts_nested_attributes_for :contributor
end

class BugTest < Minitest::Test
  def test_accepts_nested_attributes_for_and_protected_attributes
    contributor = Contributor.create

    contributor.update_attributes({ "credits_attributes" => {
      "0" => {"title_id"=> Title.create.id}, "1" => {"title_id"=> Title.create.id} }
    })

    assert_equal 2, Contributor.first.credits.count
  end
end

When I run that file I get the following

Resolving dependencies...
Using i18n 0.7.0
Using json 1.8.3
Using minitest 5.8.3
Using thread_safe 0.3.5
Using builder 3.2.2
Using arel 6.0.3 (was 5.0.1.20140414130214)
Using sqlite3 1.3.11
Using bundler 1.11.2
Using tzinfo 1.2.2
Using activesupport 4.2.0 (was 4.1.14)
Using activemodel 4.2.0 (was 4.1.14)
Using activerecord 4.2.0 (was 4.1.14)
Using protected_attributes 1.1.3
-- create_table(:titles, {:force=>true})
D, [2015-12-28T09:58:34.378354 #75582] DEBUG -- :    (0.4ms)  CREATE TABLE "titles" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL) 
   -> 0.0059s
-- create_table(:contributors, {:force=>true})
D, [2015-12-28T09:58:34.379314 #75582] DEBUG -- :    (0.2ms)  CREATE TABLE "contributors" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL) 
   -> 0.0009s
-- create_table(:credits, {:force=>true})
D, [2015-12-28T09:58:34.380149 #75582] DEBUG -- :    (0.1ms)  CREATE TABLE "credits" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "title_id" integer, "contributor_id" integer) 
   -> 0.0007s
Run options: --seed 34057

# Running:

D, [2015-12-28T09:58:34.400625 #75582] DEBUG -- :    (0.1ms)  begin transaction
D, [2015-12-28T09:58:34.404294 #75582] DEBUG -- :   SQL (0.1ms)  INSERT INTO "contributors" DEFAULT VALUES
D, [2015-12-28T09:58:34.404821 #75582] DEBUG -- :    (0.1ms)  commit transaction
D, [2015-12-28T09:58:34.406497 #75582] DEBUG -- :    (0.0ms)  begin transaction
D, [2015-12-28T09:58:34.409141 #75582] DEBUG -- :   SQL (1.9ms)  INSERT INTO "titles" DEFAULT VALUES
D, [2015-12-28T09:58:34.410096 #75582] DEBUG -- :    (0.2ms)  commit transaction
D, [2015-12-28T09:58:34.410761 #75582] DEBUG -- :    (0.1ms)  begin transaction
D, [2015-12-28T09:58:34.411422 #75582] DEBUG -- :   SQL (0.1ms)  INSERT INTO "titles" DEFAULT VALUES
D, [2015-12-28T09:58:34.413418 #75582] DEBUG -- :    (0.2ms)  commit transaction
D, [2015-12-28T09:58:34.415744 #75582] DEBUG -- :   Contributor Load (0.2ms)  SELECT  "contributors".* FROM "contributors"  ORDER BY "contributors"."id" ASC LIMIT 1
D, [2015-12-28T09:58:34.418755 #75582] DEBUG -- :    (0.1ms)  begin transaction
D, [2015-12-28T09:58:34.438892 #75582] DEBUG -- :   SQL (0.2ms)  INSERT INTO "credits" ("title_id", "contributor_id") VALUES (?, ?)  [["title_id", 2], ["contributor_id", 1]]
D, [2015-12-28T09:58:34.469862 #75582] DEBUG -- :    (0.4ms)  rollback transaction
E

Finished in 0.077507s, 12.9020 runs/s, 0.0000 assertions/s.

  1) Error:
BugTest#test_accepts_nested_attributes_for_and_protected_attributes:
SystemStackError: stack level too deep
    /Users/fjguzman/.rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/activerecord-4.2.0/lib/active_record/connection_adapters/abstract/transaction.rb:191

1 runs, 0 assertions, 0 failures, 1 errors, 0 skips

My problem is already solved since rails 4.2.1.rc1 without applying this patch but I cannot update my app to rails 4.2.x yet. We're still solving some minor issues like this one.

@rafaelfranca would you merge this change if I add some tests to it? Checking the calls to after_update callback or the one that I showed you on the report bug file? I wasn't able to find what is happening in the activerecord internals

[danielrsmith]

:+1: this fixes our problem with 4.1.x as well.

[westonganger]

+1

[arthurnn]

thanks a lot @pkmiec , #81 was merged, which included the tests.