-
Type: Task
-
Resolution: Done
-
Affects Version/s: None
-
Component/s: None
Mongoid's Object#_vacant method looks like this:
ruby
def _vacant?
is_a?(::Enumerable) || is_a?(::String) ? empty? : !self
end
This is all fun and games until someone decides to include Enumerable in the Mongoid document. In which we'd get:
undefined method `empty?' for #<CustomViews::View:0x104ff3818>
That's obviously not so great. The problem here is the flawed assumption that something that includes enumerable will respond to #empty?, even though the Ruby documentation makes no mention that it should. It's not even clear that asking this question in the first place will give a sensible result. Especailly considering the code that's asking the question here:
ruby raise Errors::DocumentNotFound.new(klass, args) if result._vacant?
In this case, result seems to be an instance of my mongoid document. If _vacant? were to return false because my document is an enumerable collection which contains no members, that would give an undesired result. Mongoid would raise an exception, claiming my document was not found, when in fact it was.
I believe the solution is to change the implementation of Object#_vacant? to something like:
ruby
def _vacant?
is_a?(::Array) || is_a?(::String) ? empty? : !self
end
But even that is quite dangerous, as someone might make their Mongoid::Document a subclass of Array, for some ungodly reason. Imo, it would be best to use Object#blank? instead and simply disallow empty as a column name. Additionally, the code in execute_or_raise should probably be made safer.
Let me know if you agree and I will work up a patch.