ActiveRecord Bug: AR .persisted? throws SystemStackError for an unsaved model with a custom primary_key that didn't save due to validation error

persisted_custom_pk_bug.rb

# AR .persisted? throws SystemStackError for an unsaved model with a
# custom primary_key that didn't save due to validation error
gem 'activerecord', '4.1.0.rc1' # in HEAD too
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 :posts do |t|
    t.string :name
  end

  create_table :comments, id: false do |t|
    t.integer :comment_id
    t.string :name
  end
end

class Post < ActiveRecord::Base
  validates_presence_of :name
end

class Comment < ActiveRecord::Base
  self.primary_key = :commentId

  validates_presence_of :name
end

class BugTest < Minitest::Test
  def test_persisted_fails_with_custom_primary_key
    post = Post.create
    # .persisted? works fine with a regular primary key
    assert !post.persisted?

    comment = Comment.create name:'foo'
    assert comment.persisted? # works fine since validations passed

    comment.name=nil # now we break validations
    assert !comment.save
    assert comment.persisted? # Works fine if it has been saved previously

    # But when a validation fails on an unsaved Model with a custom primary key and
    # then you call .persisted? you get an Exception: "SystemStackError: stack level too deep"
    comment = Comment.create
    assert !comment.persisted?

  end
end

# Cause:
#
# perisisted? calls
#   new_record? calls 
#     sync_with_transaction_state calls  <-------------------
#       update_attributes_from_transaction_state calls       |
#         restore_transaction_record_state calls             |
#           self.id= calls                                   |
#             sync_with_transaction_state AGAIN -------------
#
# So if we change the self.id= to a write_attribute(self.class.primary_key, )
# then the looping sync_with_transaction_state is skipped
#
#
# Here is self.id= from active_record/attribute_methods/primary_key.rb
# I'm assuming that if you have a primary_key named `id` this gets overridden
      # module ActiveRecord
      #   module AttributeMethods
      #     module PrimaryKey
      #       # Sets the primary key value.
      #       def id=(value)
      #         sync_with_transaction_state
      #         write_attribute(self.class.primary_key, value) if self.class.primary_key
      #       end

# After you see the above test fail,
# apply the Monkey Patch by switching the lines detailed below
# activerecord/lib/active_record/transactions.rb
module ActiveRecord
  module Transactions

    protected
    def restore_transaction_record_state(force = false) #:nodoc:
      unless @_start_transaction_state.empty?
        transaction_level = (@_start_transaction_state[:level] || 0) - 1
        if transaction_level < 1 || force
          restore_state = @_start_transaction_state
          was_frozen = restore_state[:frozen?]
          @attributes = @attributes.dup if @attributes.frozen?
          @new_record = restore_state[:new_record]
          @destroyed  = restore_state[:destroyed]

          if restore_state.has_key?(:id)
#
# Uncomment the next line and comment the following one to have the test pass
#            write_attribute(self.class.primary_key, restore_state[:id])
            self.id = restore_state[:id]
          else
            @attributes.delete(self.class.primary_key)
            @attributes_cache.delete(self.class.primary_key)
          end
          @attributes.freeze if was_frozen
        end
      end
    end
  end
end