Reification of associations is not compatible with transactional tests

[timwaters]

Hello, I'm using Test Unit / Minitest on a new Rails 4.2 application on Ruby 2.2 with Paper Trail 4.0.0.rc1. I am seeing associations being restored in the application, in the console in both postgres and sqlite dbs. However within a test the associations are not being restored properly. I wouldn't be surprised if I wrote the test wrong!! but given a search of the Issues there may be something going on.

The simple application is here: https://github.com/timwaters/paper_trail_testing and here are some of the relevant parts:

class Comment < ActiveRecord::Base
  belongs_to :article
  has_paper_trail

  after_save :touch_article

  def touch_article
    article.touch_with_version
  end
end

class Article < ActiveRecord::Base
  has_many :comments
  has_paper_trail
end

and test

require 'test_helper'

class ArticleTest < ActiveSupport::TestCase

  a =  Article.new 
  a.title = "first" 
  a.text = "text stuff"
  a.save

  c = Comment.new
  c.text = "this is a comment"
  c.article = a
  c.save

  c = Comment.last
  c.text = "changed"
  c.save

  article = Article.last

  refied_article = article.reload.versions.last.reify(:has_many => true)

  puts refied_article.comments.inspect  #<ActiveRecord::Associations::CollectionProxy [#<Comment id: 980191006, text: "this is a comment", ....
  puts refied_article.comments.first.text == "this is a comment"  #TRUE


  test "adding versioning " do
    with_versioning do

      a =  Article.new 
      a.title = "first" 
      a.text = "text stuff"
      a.save

      c = Comment.new
      c.text = "this is a comment"
      c.article = a
      c.save

      c = Comment.last
      c.text = "changed"
      c.save

      article = Article.last

      refied_article = article.reload.versions.last.reify(:has_many => true)

      puts refied_article.comments.inspect  ##<ActiveRecord::Associations::CollectionProxy []>
      assert refied_article.comments.first.text == "this is a comment"  #undefined method `text' for nil:NilClass

    end
  end

end

The first block gets executed outside of the test, and performs as expected. If I put the same code in the block the comments do not get restored. Indeed, there are no comments. (Reloading the comments gives the latest comment, as expected)

The article is reified to the correct version in each.

The same behaviour works if I out the test code outside of the with_versioning block, if Paper Trail is enabled globally, or disabled globally and just enabled in the specific test, and wrapping the changes in Article.transaction and changing the article at the same time also does not seem to make much difference. It also seems to make no difference adding PaperTrail::Rails::Engine.eager_load! to an initializer.

The test log can be found here: https://gist.github.com/timwaters/e26b4e2ae0583b3fc5a8 and to my eyes it appears as if it's doing the right thing equally in both cases, just not loading up the correct association properly. (Disabling fixtures doesn't seem to change things either)

Many thanks for your time.

[timwaters]

I'm getting the same empty associations in a controller test also. https://github.com/timwaters/paper_trail_testing/blob/master/test/controllers/versions_controller_test.rb

As with the unit test, in the actual app, associations are being restored.

Could this be related to #492 or is it about the test environment?

[jaredbeck]

Thanks for the failing tests, Tim. I've pulled down timwaters/paper_trail_testing and I'm taking a look.

[jaredbeck]

@batter Here's what I've learned running Tim's excellent unit test in a debugger.

In Tim's first block, outside of the test,

1. each create runs in a separate transaction,
2. causing each record in version_associations to have a different transaction_id.
3. When reify_has_many_directly runs its select min(version_id) .. where .. or transaction_id = ? it only finds the record representing an update.

In Tim's second block, inside the test,

1. each create runs in the same transaction (because use_transactional_fixtures is true by default),
2. causing each record in version_associations to have the same transaction_id.
3. When reify_has_many_directly runs its select min(version_id) .. it finds the create.
4. options[:mark_for_destruction] is false, so the create is discarded

For fun, try setting self.use_transactional_fixtures = false in Tim's unit test, and it'll pass.

[jaredbeck]

@timwaters Without turning off use_transactional_fixtures you could simulate separate transactions by calling PaperTrail.transaction_id=. That should make your test pass.

@batter Have you had a chance to look at this? I'm starting to wonder if this should continue to hold up our 4.0 release.

[jaredbeck]

Here's my attempt at a failing test that we could use in our test suite.

--- a/test/unit/model_test.rb
+++ b/test/unit/model_test.rb
@@ -1504,7 +1504,7 @@ class HasPaperTrailModelTransactionalTest < ActiveSupport::TestCase
     context 'updated before the associated was created' do
       setup do
         @customer.update_attributes! :name => 'customer_1'
-        @customer.orders.create! :order_date => Date.today
+        @order = @customer.orders.create! :order_date => "Thursday"
       end

@@ -1517,6 +1517,13 @@ class HasPaperTrailModelTransactionalTest < ActiveSupport::TestCase
         should 'not persist changes to the live association' do
           assert_not_equal [], @customer.orders(true)
         end
+
+        should 'demonstrate issue 542' do
+          @order.update_attributes! :order_date => "Friday"
+          assert_equal "Thursday", @order.versions.last.reify.order_date
+          reified_customer = @customer.versions.last.reify(:has_many => true)
+          assert_equal "Thursday", reified_customer.orders.first.try(:order_date)
+        end
       end

It fails with:

Expected: "Thursday"
  Actual: nil

Is this a useful failing test, or do I just not understand reified has_many associations?

[batter]

I haven't had a chance to look at this yet, I will try to take a peek and run the tests locally by EOD today

[batter]

Hmm... just read through this a little more closely, and dealing with complex nested associations and "playbacks" in transactional scenarios is indeed somewhat tricky.

So I just cloned down the sample project provided by @timwaters, and upon close inspection it appears that the gem is actually behaving as expected, and that the issue appears to be the fact that the after_save callback attached to the Comment class is being executed after a comment modification is persisted to the database, and thus the Article's new timestamp (and thus also the version generated by the call to touch_by_version) is timestamped slightly after the time the comment's timestamp update has been registered.

If you wanted to make your setup work, I would recommend changing your Comment class to look something like this:

class Comment < ActiveRecord::Base
  belongs_to :article
  has_paper_trail

  after_save :touch_article

  def touch_article
    article.update_attribute(:updated_at, self.updated_at)
  end
end

That would ensure the timestamp will match for both the latest Comment and Article records... that being said, I just gave that a whirl and that also doesn't appear to be reifying as I would expect when executed in the console in sandbox mode (where everything is run inside of a transaction), so this does indeed appear to be an issue surrounding transactions and associations.

[jaredbeck]

I've added a warning to the readme: ce37034

[jaredbeck]

Update:

• Reification of associations is not compatible with transactional tests, aka. transactional fixtures.
• This is an issue we'd like to fix, but are stumped.
• We welcome suggestions ❤️

[quolpr]

One of possible workaround if you are using database_cleaner:

RSpec.configure do |config|

  config.use_transactional_fixtures = false

  config.before(:suite) do
    DatabaseCleaner.clean_with(:truncation)
  end  

  config.before(:each) do
    DatabaseCleaner.strategy = :transaction
  end

  config.before(:each, versioning: true) do
    DatabaseCleaner.strategy = :truncation
  end

  config.before(:each) do
    DatabaseCleaner.start
  end

  config.append_after(:each) do
    DatabaseCleaner.clean
  end

end

Didn't test it, but may work.

[jaredbeck]

Closing due to inactivity. This will remain listed in the readme with the other many known issues surrounding the experimental association tracking feature.

[icklecows]

We have been able to work around our issues with this by using the Transactional Capybara gem - it may not be generally applicable, but it resolved our issues. We use it for testing published vs draft versions of models.