Saving a new node after adding a parent fails validation

[MarkMT]

This issue is somewhat related to #11 which concerned the addition of an existing node as a parent to a new unsaved node. Commit #17 resolved this with some validation changes in the link class and additional options in some has_many calls on the node class.

However #11 did not address what happens when the new node is subsequently saved. While upgrading a Rails 3.2.x app to 4.x, I discovered that saving a node in this situation actually fails despite the fact that valid? returns true.

> a = Person.create
> b = Person.new
> b.parents << a
> b.valid?
=> true
> b.save
=> false

The problem manifests in slightly different ways depending on whether the parent is a terminal or has a parent itself. Although I have only tested on Rails 4.0, I believe the issue remains present in subsequent versions and occurs in part because of a change in Rails 4.x in the implementation of ActiveRecord::Associations::HasManyThroughAssociation#concat (see: rails/rails@610b632). This method calls concat_records which has a new implementation in the HasManyThroughAssociation class. One thing this new implementation does is to build a new through record, i.e. a link object. This object is instantiated when parents are added to the node record regardless of whether that record has been persisted. In Rails 3.x, in contrast, the through object is not built until the child is saved.

In the process of building the through object, the Rails 4 implementation also instantiates a 'through association' object, i.e. representing the has_many :links_as_child association. This change leads to two different problems for acts-as-dag...

1. When the child is saved, the through object is saved twice, causing acts-as-dag's UpdateCorrectnessValidator to fail the 'no changes' test. This occurs as follows. Activerecord autosaves the record's associations (see AutosaveAssociation::ClassMethods.add_autosave_association_callbacks) including the has_many :links_as_child and has_many :parents, :through => :links_as_child associations:
   1. has_many :links_as_child is the through association that was instantiated when the parent was added to the child prior to saving. Because the association has been instantiated, saving it results in the through object, the link connecting the parent and child nodes, being persisted. This does not happen in Rails 3.x because at this point the association had not yet been instantiated.
   2. The has_many :parents, :through => :links_as_child association is saved after has_many :links_as_child, and this also causes the through record to be saved again (see HasManyThroughAssociation#insert_record). This in turn causes acts-as-dag's UpdateCorrectnessValidator validation to fail because the record has not changed since the last time it was saved.
2. A second, different problem arises if the parent itself has a parent, with the result that the bridging link between child and grandparent fails to validate. This occurs as follows. Prior to autosaving the child object's associations as described above, all of those associations are first validated (these validations are also added by AutosaveAssociation::ClassMethods.add_autosave_association_callbacks). In the case of the has_many :links_as_children association, acts-as-dag's CreateCorrectnessValidator checks whether the link has any duplicates by calling has_duplicates(record). This method calls source and sink on the record, which instantiates and memoizes a pair of EndPoint objects based on the record's current ancestor and descendant. However since the child has not yet been saved, the sink endpoint is memoized with a nil id. Once validations have completed the associations are autosaved. However saving the link association triggers the perpetuate before_save filter. If the parent itself has a parent, perpetuate will try to create a new indirect bridging link between the child and the grandparent. However it creates this object using the previously memoized sink from the link between the parent and child. As a result, the bridging link is instantiated with a nil descendant_id and therefore when it is saved it fails the validates :descendant, :presence => true check.

I have a PR coming that resolves both of these issues. The first problem requires simply removing the "No changes" test from UpdateCorrectnessValidator. I don't see a compelling reason to retain this test since it is really a concern of activerecord. The second problem can be resolved by circumventing memoization of the source and sink if they point to records that haven't yet been persisted.

Also, in the course of testing this I discovered a flaw in one of the existing tests:

  #Tests that we catch links that would be duplicated on creation
  def test_validation_on_create_duplication_catch
    a = Node.create!
    b = Node.create!
    e = Default.create_edge(a, b)
    e2 = Default.create_edge(a, b)
    assert !e2
    assert_raises(ActiveRecord::RecordInvalid) { e3 = Default.create_edge!(a, b) }
  end

This will never fail the create validation since create_edge itself finds the existing link and updates it. The solution is to replace it with build_edge(a,b).save.