mongoid/mongoid — Issue #1349: Epic fail when Mongoid documents include Enumerable
Author: jnicklas
Created: 2011-10-17 13:14:03+00:00
State: open

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 #
```

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](http://www.ruby-doc.org/core-1.9.2/Enumerable.html) 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](https://github.com/mongoid/mongoid/blob/master/lib/mongoid/criterion/inclusion.rb#L151):

``` 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.