Transaction

[cambridgemike]

This fork is 99% the work of @egoh, I just added some unit tests for the has_many and fixed a few minor bugs. A few things to note:

1. Because the default behavior of the tests is to run everything inside the setup block as a single transaction, a lot of the reify association stuff was breaking (since it depends on transaction IDs). This was addressed before by using the method "make_last_version_earlier", I tried using the same kind of hack, but couldn't get it to work. Ultimately I just turned off the transactional fixtures.

2. I question how the reify_has_manys and reify_has_ones handled the create event. It seemed to me like they should have set child equal to nil (line 172 and 200 of version.rb), but if I'm missing something then please correct.

3. I had to make some of the inheritance column tests more explicit since the transactional fixtures was turned off.

4. I only wrote some very basic tests for has_many, but now that they're passing it should be easier to write some more.

[cambridgemike]

@airblade did you get a chance to give this a try? Let me know if you want me to add some more tests.

[cambridgemike]

My app was being plagued by a pretty nasty bug, where when you reify a has many association, new versions were being created of the child object. Apparently "<<" calls save on the object almost automatically.

I was finally able to write some tests that catch that behavior in the act. Just committed these (sorry about the dirty commit history). @egoh or @airblade if you have any idea how to fix this, I'd be pretty curious. I'm quite stumped.

[cambridgemike]

I should add, I'm usin rails 3.05. It seems that associations are handled differently in 3.1 and 2.3.

[tderks]

@mike I've been really busy lately with my education and other programming work, so have no real time to put into this. I'm glad someone picks it up to try get it in the trunk release! I noticed the problem before and i have a tip for you!
You can build the has many association in memory with the assoc_name.build(attributes={}) method. You can't insert the attributes directly, since it will block out all the protected attributes. Therefor the same trick as in the reify method needs to be used. Hope this gives you enough info.
If you have any other questions just ask, i have spent some time trying to get it all working, but new things just kept popping up...

Another thing, the added flexibility airblade recently introduced with respect to model/tablename flexibility/extension is really nasty to extend to the whole association part.

[cambridgemike]

Thanks for the tip! I tried that a while back, but didn't have unit tests to guide me. I'll have another go at it later tonight using this approach.

[tderks]

@mike That could be right, i think it had worked in some rails version (don't know which one i used back then).
But the build method is the way to go for future rails versions as stated in the documentation: http://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html (Unsaved objects and associations section).
Reading the docs the has_one association should have the same problem and could be fixed in same way.

[cambridgemike]

@egoh. This is pretty kludgy, but it passes the tests.

      child = version.reify(options)
      new_elm = model.send(assoc.name).build child.attributes
      new_elm.id = child.id

However, I'm pretty sure this will break horribly with nested has_many. But I need to write some tests for that

[tderks]

@mike Like i said before, this will not work with protected attributes.
And yes, this will not restore any nested models

[cambridgemike]

After weighing the options, I'm not a fan of using the .build approach, since we'll always have to do a LOT of heavy lifting in order to get nested models working. The other option is to find a way to prevent << from executing a 'save' on the child. Digging in to the specifics of rails 3.0x it seems like the child is not saved if the parent object is a new record (checked with new_record?). We can fake it out by doing this

      child = version.reify(options)
      model.instance_eval { alias :old_new_record? :new_record? }
      model.instance_eval { alias :new_record? :present? }
      model.send(assoc.name) << child
      model.instance_eval { alias :new_record? :old_new_record? } 

A similar approach, which would be more resilient to changes in rails, would be to intercept the DB writes, and prevent them.

[cambridgemike]

@egoh, I'm running into the same sort of problem, but in the delete case (when the version is a 'create' event). In your original pull request you had

 child.mark_for_deletion
 model.send(assoc.name) << child

How was this intended to work? Wouldn't the child still be in the collection, just deleted once you save the parent?

[tderks]

@mike yup, cause that was the easy way to go.

The alternative better way is to really delete it from the collection, but then you need to store the childs that need to be deleted somewhere in the parent object and remove them when you safe the object.

[cambridgemike]

@egoh, @airblade. Yikes, I've found another use case that fails... thoughts on this one?

    # Widget is created with two children
    @widget = Widget.create :name => "widget_0"
    @glimmer_a = @widget.glimmers.create :name => "glimmer_0a"
    @glimmer_b = @widget.glimmers.create :name => "glimmer_0b"

    # Widget is updated, and one of the children is updated
    @widget.update_attributes :name => "widget_1"
    @glimmer_a.update_attributes :name => "glimmer_1a"

     # Widget is reified
    @widget_0 = @widget.versions.last.reify(:has_many => 1)

     # @widget_0 should still have two children (right?). This fails, it only has 1
    assert_equal 2, @widget_0.glimmers.length

The problem is that during reify we're only appending the children that have versions after the last widget version. I wonder if there is a completely different approach to working with these association collections...

[cambridgemike]

I think I might have found a clever way to bypass a lot of this mess. AssociationProxy has a target= method which lets you set the target collection directly. The has many reify would then look something like this. Using this we can start with the pre-reified collection, make changes to it based on the versions, then set it as the collection on the reified object.

  # Pass true to force the model to load
  collection = Array.new model.send(assoc.name, true)

  versions.each do |version|
    if(version.event=='create')
      if(child=version.item)
        collection.delete child
      end
    else
      child = version.reify(options)

      collection.map! do |c|
        c.id == child.id ? child : c
      end

    end
  end

  model.send(assoc.name).target = collection

[batter]

Closed by #439, which appears to have been heavily influenced by this PR and #44. Cheers!