Uploaded image for project: 'Mongoid'
  1. Mongoid
  2. MONGOID-1350

Epic fail when Mongoid documents include Enumerable

    • Type: Icon: Task Task
    • Resolution: Done
    • 2.3.3
    • Affects Version/s: None
    • Component/s: None
    • Labels:

      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.

            Assignee:
            Unassigned Unassigned
            Reporter:
            jnicklas Jonas Nicklas
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

              Created:
              Updated:
              Resolved: