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' )
or
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) 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 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?
-
-
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.
Create new ticket
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
Dan Kubb (dkubb)
Rick