#459 ✓resolved
Aadi Deshpande

DM::Query::Path should undef type and id

Reported by Aadi Deshpande | July 16th, 2008 @ 01:30 AM

If these two properties are not undefined, then type and id get evaluated before method_missing has a chance to do its' magic and build the query relationship chain.

Without that, queries like

User.all( User.book.type => 'fiction' )


User.all ( User.book.author.id => 1 )

In my work, we use type and id a lot ( id significantly more so, as I'm sure a lot of people do )

I've attached a patch of edge which updates the spec to include this scenario in a failing condition, and a simple resolution to it.

Comments and changes to this ticket

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) July 16th, 2008 @ 04:08 AM

    • State changed from “new” to “open”

    Good catch.

    Query::Path should probably undef most methods, aside from a few key ones needed by ruby internals (__send__, respond_to?, etc). The OneToMany::Proxy class has an example where this is done.

  • Aadi Deshpande

    Aadi Deshpande July 16th, 2008 @ 09:36 AM

    i used that one as a baseline, but in this scenario, i felt far safer using a small blacklist instead of a large whitelist.

    the whitelist that makes all the specs pass is :

    instance_methods.each { |m| undef_method m unless %w[ __id__ __send__ class kind_of? respond_to? assert_kind_of should should_not instance_variable_set instance_variable_get hash send nil? eql? to_sym ].include?(m) }

    however, that whitelist still doesn't work in my code, when dealing with enums ( it fails on 'relationship' property )

    i think we should stick with the blacklist until there's a strong case made for other properties that need to be in there.

    I specifically picked this blacklist because they have non-deprecated equivalents ( type => class, id => object_id )

    what do you think?

  • Aadi Deshpande
  • Bernerd Schaefer

    Bernerd Schaefer July 22nd, 2008 @ 11:22 AM

    • State changed from “open” to “resolved”

    Thanks for the fix!

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

People watching this ticket