Wednesday, April 12

has_many: apparently doesn't have enough

These last couple weeks I've been making blazing progress on my pet Project Basement. I've fallen even more in love with Ruby on Rails, as if that was possible. You know the kind of sick infatuation when you start thinking to yourself, "I wonder how long it would take me to rewrite my company's six-year-old one-hudred-seventy-five-thousand-line Java application in Rails?" That's how sick it is. But I'm already off track before I've even begun. So on to my latest rant.

As the title implies, I've been a little disappointed with "has_many" as of late, specifically with the management of the "through" cross-reference records. For example, I've got the following models:

class Collectible < ActiveRecord::Base
end

class Collector < ActiveRecord::Base
has_many :possessions
has_many :collectibles, :through => :possessions
end

class Possession < ActiveRecord::Base
belongs_to :collector
belongs_to :collectible
end

Simple enough, right? If I want a list of all the Collectibles possessed by a Collector, I just call "collector.collectibles" and the back-end does all the magic for me. It's a thing of beauty.

But what if I want to add a new Collectible to a Collector? This is where things get ugly. I'm hoping that this is simply a misunderstanding on my part and somebody will set me straight, but I've been banging my head against this for the last couple night and it's not adding up.

According to the documentation for has_many:

collection<<(object, …) - adds one or more objects to the collection by setting their foreign keys to the collection’s primary key.

That seems to imply that I should be able to call "collector.collectibles << some_collectible" and the new collectible will automatically be added to the list and the appropriate database record created, but that's not what's happening. Here's one of my test cases from "test/unit/possession_test.rb":

def test_collector_collectibles
ted = Collector.find(1)
nickel = Collectible.find(2)

assert ! ted.collectibles.include?( nickel )
# add a collectible to a collector
ted.collectibles << nickel

# sanity check
assert ted.collectibles.include?( nickel )

ted = nil
ted = Collector.find(1)
assert ted.collectibles.include?( nickel )
end

That last assertion fails! When I reinitialize the Collector, forcing it to reload all it's guts from the database, the Collectible I just added to his Possessions isn't there. It apparently wasn't committed to the database.

I considered perhaps I needed to explicitly save the change, so I tried "ted.save" but that did nothing and "ted.collectibles.save" threw an error as the method isn't implemented.

Now, if I deal directly with the "possessions" list I can make it work, but I don't like the syntax at all:

def test_collector_possessions
nickel = Collectible.find(1)
ted = Collector.find(3)
assert ted.possessions.find_by_collectible_id( nickel.id ).nil?

ted.possessions.create( { :collectible_id => nickel.id } )

ted = nil
ted = Collector.find(3)
assert ! ted.possessions.find_by_collectible_id( nickel.id ).nil?
end

That, in my humble opinion, is butt-ugly by Ruby and Rails standards. I contend that my first test, the one with the simple and elegant syntax, should work!

I've been in this situation before, where I've found myself frustrated with something in Ruby on Rails, but I've almost always eventually hit the "A ha!" moment where it all made sense. But, this time, it ain't happening. Can somebody out there explain and/or justify this disconnect?

10 comments:

Trevor Squires said...

Hey,

as far as I know, :through associations are read-only.

It makes sense because the model you are going :through is completely standalone and as such, it may have more than just the two belongs_to associations you specify.

I.e. the has_many :through directive doesn't capture enough information to reliably create the model you are going :through.

Makes sense?

Trevor

Trak3r said...

> as far as I know, :through associations are read-only.

It seems that way, now that I've delved into the code (see my follow-up post).

> It makes sense because the model you are going :through is completely
> standalone and as such, it may have more than just the two belongs_to
> associations you specify.

Hmm, that may be, but I'm not entirely convinced of your argument. Rails is very much a "features that work for most cases" framework and in most cases the association is going to contain only the two foreign keys.

> I.e. the has_many :through directive doesn't capture enough information
> to reliably create the model you are going :through.

It captures enough information to pull the relationship out of the database, so it theoretically has enough information to put a relationship back into the database (barring, as you point out, if the relationship has any extra references). I still believe this can and should work, and I hope to get to the bottom of this soon.

Thanks for your feedback, Trevor.

Trevor Squires said...

Hey,

wrt your assertion that in most cases the relation will contain just the two foreign keys:

Then that's not a has_many :through, it's a has_and_belongs_to_many. The :through directive was added to eliminate the grief associated with having additional attributes in the join table for a habtm.

So if you're *only* going to have two belongs_to associations then your life will probably be much simpler if you just go for has_and_belongs_to_many - that's what it's designed for.

Regards,
Trevor

Trak3r said...

Trevor, thanks for the note, but it ran me into another road block. I switch to a HABTM in my Collector model:

has_and_belongs_to_many :widgets, :class_name => 'Collectible', :join_table => :desires

Then I created a new unit test:

def test_habtm
nickel = Collectible.find(1)
ted = Collector.find(3)
assert ! ted.widgets.include?( nickel )
ted.widgets << nickel
end

And I get this screwy error when I run it:

1) Error:
test_habtm(DesireTest):
ActiveRecord::StatementInvalid: Mysql::Error: #23000Duplicate entry '1' for key 1: INSERT INTO desires (`created_on`, `updated_at`, `collectible_id`, `created_by`, `updated_by`, `id`, `collector_id`) VALUES (NULL, NULL, 1, NULL, NULL, 1, 3)

What's it's doing, as I discovered through trial and error, is replacing the 'id' value being inserted into the Desires table with the Collectible ID value. This table technically doesn't need an ID column since it's just a two-way cross-reference so I removed it from the migration (which creates it by default), regenerated the database, and got a completely different error because the framework still attempted to populate that [now missing] column.

So it appears that it is supposed to be working now, but it's not generating the correct SQL, and therefore not really working.

It's hard to tell if I'm a little bit closer or a little bit farther from my destination now :-)

Thanks.

Trevor Squires said...

Hrm, it should be very straight forward.

My migrations:

def self.up
create_table :collectors do |t|
t.column :name, :string, :limit => 128, :null => false, :default => ''
end

create_table :collectables do |t|
t.column :name, :string, :limit => 128, :null => false, :default => ''
end

create_table :desires, :id => false do |t|
t.column :collector_id, :integer
t.column :collectable_id, :integer
end
end

And in collector.rb:

class Collector < ActiveRecord::Base
has_and_belongs_to_many :widgets, :class_name => "Collectable", :join_table => :desires
end

And in collectable.rb:

class Collectable < ActiveRecord::Base
has_and_belongs_to_many :collectors, :class_name => "Collector", :join_table => :desires
end

In the console I type:

me = Collector.create(:name => 'me')
mything = Collectable.create(:name => 'my thing')
me.widgets << mything
me.widgets << Collectable.new(:name => 'another thing')
me.widgets.first.collectors.first

And it all looks good. So what are you doing differently?

Trak3r said...

Hey Trevor. Thanks for the note.

I only notice two differences between your test code and mine:

1. You added the tag ":id => false" to your migration for the xref table.

2. You included the reverse relationship declaration in your Collectible model (I didn't think this was necessary, and still don't).

I added ":id => false" to my migration, regenerated the database, and ran a similar test from the console and sure as heck it worked just fine!

>> me = Collector.create( :username => 'ted' )
>> mything = Collectible.create( :name => 'my thing' )
>> me.widgets << mything
>> id = me.id
>> mecopy = Collector.find(id)
>> p mecopy.widgets

However, when I raked the unit tests I got this error:

2) Error:
test_habtm(DesireTest):
ActiveRecord::StatementInvalid: Mysql::Error: #42S22Unknown column 'id' in 'field list': INSERT INTO desires (`id`) VALUES (2)

This seems to suggest that the new database schema isn't getting properly raked from development to testing. Just to be sure, I dropped the testing database completely, recreated it, and still got the same error :-\ I'm at a loss as to why this code would work in the console and not in the unit test.

Trevor Squires said...

Hey,

yeah, I'm at a loss too. The code that I've used works equally well in unit/functional tests as it does from the console (both rails 1.0 and 1.1).

It's all pedestrian stuff as far as ActiveRecord is concernned - been stable for absolute ages.

Knowing that this *should* work, and assuming that you're posting 'sanitised' code pulled from something larger here I'd suggest the following:

Start from a clean rails project, 3 new and empty tables in a clean database, and 2 new models that have nothing but the habtm defined (on both sides).

Stick to naming conventions so you don't have to specify :join_table or :class_name.

I suspect once you do that it'll 'just work' and from there you can compare it to the version you've got today.

Trev

Trak3r said...

Thanks again, Trevor. I didn't get around to trying a clean test as you suggested, but I did finally work out a rather elegant solution to what I originally wanted. Check out my latest post and let me know what you think ;-)

P. S. Are you going to be at RailsConf2006? I'm looking to meet some good Railers in person.

Jim said...

I found this to be useful:

http://tuples.us/2006/11/14/has_many-through-accessor-methods/

Jim said...

^-- actually, I meant this one:

has_many_through_assignable