Agilefreaks / Aquarium

An AOP library for Ruby
https://rubygems.org/gems/aquarium/
148 stars 24 forks source link

Aquarium::Finders::TypeFinder#empty #37

Open paulwalker opened 11 years ago

paulwalker commented 11 years ago

I am not sure what the purpose of the empty? part of this empty method is, but it may cause an issue when creating aspects on types that use that method for other purposes than what I believe is what is intended here. For example. MongoMapper document types use it to query the document collection, which actually raises a NameError when the connection has not been set yet (which is likely when aspect code interpreted). Consider removal of the empty? half of this condition?

deanwampler commented 11 years ago

It's only used within TypeFinder at two point to weed out empty strings and regular expressions. It should never be called on other types or objects that you might be advising. It, and most of the other helper methods, should be protected, though.

paulwalker commented 11 years ago

Hmm...I don't see any type checking in the condition and #empty? is actually happening on the class I defined in the Aspect. Could it be because I am not using an array on on_types?

opts = { :calls_to => [method_name], :on_types => self }
Aquarium::Aspects::Aspect.new(:around, opts) do |jp, object, *args|
deanwampler commented 11 years ago

It's true that no type checking is done.

With ":on_types => self", are you trying to match all instances of the enclosing type or just the current instance? If the former, try changing "self" to the class name (even if you don't want to hard code this, it would be interesting to see if it avoids the issue).

If the latter, what happens if you change :on_types => self to :on_objects => self?