Polymorphic STI Has One Bug Strikes!

Posted by dave
on Tuesday, September 29

Just ran across one of the bugs related to polymorphic single-table inheritance and has_one relationships in ActiveRecord. I’m not totally sure, but I suspect it’s related to this one.

Given a set of (simplified) classes set up like this:

def User < ActiveRecord::Base
end
def Customer < User
  has_one :delivery_address, :as => :addressable
end
def Address < ActiveRecord::Base
    belongs_to :addressable, :polymorphic => true
end
def DeliveryAddress < Address
end

Associating them worked fine:

>> customer = Customer.make

>> delivery_address = DeliveryAddress.make

>>customer.delivery_address = delivery_address

And then the freakiness started:

>> customer.valid? # => true

>> customer.delivery_address.valid? # => false

>> customer.delivery_address.addressable.valid? # => false

>> customer.delivery_address.addressable.errors.full_messages

# => ["Delivery address can't be blank"]

At this point I was thinking, in capital letters, “WTF?”. A bit of poking around in the console revealed a clue:

=> #<DeliveryAddress id: 35, firstname: "Tom", lastname: "West", address1: "7308 Mohr Roads", address2: nil, city: "Beahanport", state_id: nil, zipcode: "sectorBR", country_id: 250, phone: "1-726-092-6627", created_at: "2009-09-29 14:58:21", updated_at: "2009-09-29 14:58:21", addressable_id: 9, addressable_type: "User", state_name: nil, type: "DeliveryAddress">

The addressable_id and addressable_type are both wrong – basically the DeliveryAddress can’t figure out what it’s attached to. Object reloading after saves results in a disassociated DeliveryAddress wandering around like something out of a Dostoyevsky novel.

Not having time today to fix Rails, my rather inelegant solution was to move the association up into User:

def User < ActiveRecord::Base
  has_one :delivery_address, :as => :addressable
end
def Customer < User
end

As Paul Graham once said:

SUVs, for example, would arguably be gross even if they ran on a fuel which would never run out and generated no pollution. SUVs are gross because they’re the solution to a gross problem. (How to make minivans look more masculine.)

So, in the same way, it’s a solution to a gross problem – but it works. The other (also crappy) alternative would have been to split out the Customer to not inherit from User, or perhaps to split out DeliveryAddress so it didn’t inherit from Address. This would be all out of proportion to what we’re trying to do (changing the inheritance hierarchy, adding database tables).

Dan had a sneaky solution to making sure that the lame delivery_address association never got used on the User:

validate_inclusion_of :delivery_address, :in => nil

This wins the sneaky code competition for the day, hands down.

Comments

Leave a response