Support for :inverse option in associations

The concept motivating this patch is huge -it goes right to the heart of object persistence and identity. I'm glad that Murray has put the issue on the table, but my instinct tells me that this issue needs to be addressed comprehensively or not at all. So, either AR supports...

User.first.object_id == User.first.object_id

or we accept that AR is not a transparent object persistence layer but rather an opaque Object-Relational Mapping layer.

ActiveRecord is what it is. I look forward to the merbification of Rails in this respect and I hope that other persistence layers will eventually be as practical with Rails as ActiveRecord now is.

-1

Chris,

While I appreciate you taking the time to post your comments here, I can't accept your absolutist approach.

There are dozens of real world scenarios that would benefit HUGELY from bidirectional associations, and several cases which make a full identity map much harder than you seem to think. In short:

  u = User.find(1)
  # another process changes users where id = 1
  u2 = User.find(1), :select=>"users.*, MAX(sessions.updated_at)", :joins=>:sessions)

Should we still return the same instance in this case? Or do we raise an exception?

There's a reason that hibernate has attach/detach API calls each of which throw several state-related exceptions.

There's a lot of merit to fixing this problem which is real and painful today, rather than holding off till we have a perfect solution to every possible corner case.

Koz, I in no way meant to imply that a full identity map was easy -quite the contrary. I see it as a very hard problem.

I think we disagree on the huge benefit of a partial solution. But since I'm not the one doing the work, I'm pretty poorly placed to object.

Consider my -1 as an expression of "I wouldn't want to have to do it."

The inverse on associations will mean that walking associations will always return the same instance. So things like

user.posts.first.user.posts.map(&:user).first.posts.first.user will only fetch user and posts once. Seems like a pretty big win :)

Why is :inverse specified explicitly? In your example, it's always the same as the class declaring the association.

Chris H. I actually think that a full identity map is a separate goal to supporting inverse associations. Inverse associations might open the door to get people thinking about identity maps, but I don't think we should wait for a full identity map solution before going ahead with inverses.

It might be worth describing the use-case I wrote the plugin for:

class ProjectPlan < ActiveRecord::Base
  has_many :iterations, :validate => true

  validate :duration_isnt_too_short

  def duration_isnt_too_short
    (self.end_date - self.start_date) > 3.days
  end

  def duration_range
    (self.start_date..self.end_date)
  end
end

class Iteration < ActiveRecord::Base
  belongs_to :project_plan

  validate :my_duration_is_within_project

  def my_duration_is_within_project
    self.project_plan.duration_range.include?(self.start_date) && 
    self.project_plan.duration_range.include?(self.end_date)
  end
end

Imagine we have a project plan with some iterations and we change the duration of the plan so that one or more of it's iterations are no longer valid (their durations are outside the plan). Without some kind of inverse association shared-instance stuff going on the validation for the iterations will always pass as they'll be using the DB version of the project plan that has no changes, and that's no fun.

Chris K. you're of course right. The original plugin I wrote tried to guess the inverse, but after a chat with Koz I decided to make the first cut at getting this into rails as non-magical as possible. Maybe there's a better way though, "convention over configuration" and all that, we could just assume that in the lack of an :inverse option that the default is the same as the class declaring the association.

However, because it doesn't work on every association type (yet!) maybe there is merit in making it explicit (sorry fingers!) so as not to confuse?

1+

A partial solution is definitely welcome !

I like the :inverse option since it makes this feature explicit and avoids breaking parts that do not need it.

My use case (set defaults in interdependent models):

class Node < ActiveRecord::Base
  has_many :versions
  has_one  :redaction, :class_name => 'Version', :conditions => ...
  accepts_nested_attributes_for :redaction
  before_create :set_default_name
  
  def redaction_attributes=(attrs)
    if @redaction = redaction
      @redaction.build(attrs)
    else
      @redaction = versions.build(attrs)
    end
  end
  
  private
    def set_default_name
      self.name = @redaction.title.to_s.url_name if @redaction && self.name.blank?
    end
end

class Version < ActiveRecord::Base
  belongs_to :node
  before_create :set_default_title
  
  private
    def set_default_title
      # without something like parental control, this does not work.
      self.title = node.name if self.title.blank?
    end
end

After an extended holiday (then a big hole to work my way out of after it), I'm finally back on deck enough to have a look at this.

Fundamentally this all looks pretty damned good and I'm sorry I missed the boat for 2.3

I'm wondering if the option should instead be :inverse_of instead of :inverse. So that the inverse method can return the actual association rather than just the name of it?

Also is there a reason you're using alias method chain in inverse_association.rb? From my point of view this stuff is fundamental and should just be 'wired in' to the source rather than added metaprogrammatically after the fact.

On the whole the tests look really good, but I'm wondering if we should be a little defensive when passed an explicit :inverse?

Finally, I think setting one side as an inverse should do the same to the other side, unless you can think of a reason not to?

Great work, and sorry again that I dropped off the face of the earth :)

Keep in mind that "inverse" could have two meanings in this context. The first being the other end of the association as in Murray's plugin. The second being the set of models NOT being in the association (true set inverse). I've actually written a plugin for the latter, which allows you to do things like user.magazines.subscribed.inverse to get a list (association proxy) of magazines to which the user is NOT subscribed.

Other terms that might convey the same idea without implying set inversion: complement, opposite, reflection (nice, but carries some baggage), mirror, etc.

-The absolutist

Inverse is a fairly standard term when dealing with ORM associations. Hibernate in particular uses it all over (it's where I got the term from). I don't think most of our users are thinking set theory when working with AR ;)

Michael Koziarski. I'll put together a new patch with your suggestions. However, just to clear some of them up:

1. I'll use :inverse_of instead of :inverse as the option (I like this, I wasn't keen on :inverse).
2. I like having the .inverse method of the reflection actually return the reflection object of the association rather than just the name, so I'll do that too.
3. I'll get rid of inverse_associations.rb and put the code directly into the other relevant associations files. I kind of liked having all the inverse stuff in one file (although, it's really not as I've had to patch some associations directly), but I think this was just an artefact of my pulling the patch directly out of a plugin (and being lazy).
4. When you say "be a little defensive" I presume you mean it should complain if there is no such association on the associated model? Sound's sensible to me.
5. I'm not sure about setting the inverse of the other side automatically. Given:

class ProjectPlan < ActiveRecord::Base
  has_one :gantt_chart
end

class GanttChart < ActiveRecord::Base
  belongs_to :project_plan, :inverse_of :gantt_chart
end

Right now, the gantt_chart association on ProjectPlan doesn't know that it has an inverse, but the project_plan association on GanttChart does. You're suggesting that even though we didn't explicitly tell ProjectPlan's gantt_chart association that it was an inverse of GantChart's project_plan it would know anyway because we did tell GantChart's project_plan that it was the inverse of ProjectPlan's gantt_chart.

I wonder if that's too magical? What we'd be saying is we require an explicit invocation of :inverse_of on our association definitions before we'll do anything, but if you've done that we might then add inverse information to some other associations that you've not explicitly specified the :inverse_of options for. That seems inconsistent to me.

Anyway, I'll knock up a patch with 1-4 in it and see what you (or anyone else) thinks about 5.

Sounds great Murray.

I'm all for punting on magic for a while as we let the explicit declarations get played with. I do want to make sure that 3.0 ships with some niceness (auto-finding inverses for simple cases, etc). But we can wait till we know that what we do with those inverses is safe.

And yeah, That's what I meant by 4.

I've reworked the patch. The one thing that I'm not sure about is that now it throws an exception if you supply an :inverse_of that doesn't exist, but it just silently ignores it if it's a :belongs_to that inverses onto a :has_many (the inverse type I don't have a good idea for supporting). I wonder if in this case it should also complain?

This of course opens up a whole other set of questions about how and when AR should complain about what the programmer is doing. Should it complain when AR can work out that the :inverse_of is wrong (wrong macro, wrong class, &c). Of course, if it did that for inverses, where else should/could AR be more hand-holdy?

Anyway, this new patch should answer everything Koz suggested above.

Moar eyez plz!

So the only thing which 'surprised' me playing around with this patch was this snippet:

>> t = Thing.first
=> #<Thing id: 1, user_id: 1, name: "First thing", created_at: "2009-05-04 16:35:37", updated_at: "2009-05-04 16:35:37">
>> t.object_id
=> 18883980
>> t.user.things.map(&:object_id)
=> [18753270]

GAH, preview would be nice ;)

>> t = Thing.first
=> #<Thing id: 1, user_id: 1, name: "First thing", created_at: "2009-05-04 16:35:37", updated_at: "2009-05-04 16:35:37">
>> t.object_id
=> 18883980
>> t.user.things.map(&:object_id)
=> [18753270]

I'm not sure whether it'd be feasible to make this work with just associations code, probably not.

But everything else looked awesome and worked as I expected.

I think this might be because #object_id is one of the few methods that AssociationProxy doesn't delegate:

http://github.com/rails/rails/bl...

Right, ignore the use of proxy_target, that's fine. It's the has_many side of things that's not working right. But I don't think that's a fixable issue.

Yeah, with a bit more thought I get the point your code snippet makes.

Like you though, I can't really come up with a way of making a belongs_to with an inverse of a has_many make any sense. At least, not without doing something incredibly complex that borders on a full identity map solution and I'm not sure about the full implications of that.

Apart from that, glad you liked it!

Yeah, I think we can punt on that until / unless someone does a full identity map solution.

Great job. http://github.com/rails/rails/co...

Could this be applied to 2.3.x ?

The following fix depends on it, and it would be great if these could appear as 2.3 fixes at some point:
#2815 nested models: build should directly assign the parent

No, this is a feature addition (and a fairly large one) so we can't
really push it to 2-3-stable.