search

mongoid / origin

A Ruby DSL for building MongoDB queries
http://mongoid.org/en/origin/index.html
MIT License
62 stars 29 forks source link

BUG? Mergeable#__multi__ is prematurely converting Criteria to Documents #53

Closed al closed 11 years ago

al commented 11 years ago

I've encountered what I think might be a bug in Origin. See the stack trace at the bottom of this ticket.

I'm arriving at it through a slightly unusual set of CanCan abilities, but I don't think they are the issue.

A minimal app demonstrating the problem is here: https://github.com/al/mongoid_cancan_bug. To reproduce run:

$ bundle && rake db:seeds && rails s

Then visit http://localhost:3000/profiles.json. You should see the error.

It arises from the fact that I have two applicable rules in abilities.rb. This is easily confirmed by removing one or other of the rules.

Digging through the stack trace lead me to this line in the Mergeable#__multi__ method.

The method is receiving an array of Criteria, however as soon as flatten is applied the Criteria are being resolved to actual matching documents (in this case Profile). This seems to surprise the rest of the method which evidently expects to still have an array of Criteria.

Removing the flatten will progress execution to line 134, where something similar seems to be happening.

Is this a bug or is there something unacceptable about the Criteria I am sending Mongoid/Origin's way via my CanCan abilities?

Cheers

NoMethodError (undefined method `inject' for #<Profile:0x007fbff1df7b28>):
  mongoid (3.0.9) lib/mongoid/attributes.rb:225:in `method_missing'
  origin (1.0.9) lib/origin/mergeable.rb:137:in `block (2 levels) in __multi__'
  origin (1.0.9) lib/origin/mergeable.rb:132:in `each'
  origin (1.0.9) lib/origin/mergeable.rb:132:in `block in __multi__'
  origin (1.0.9) lib/origin/mergeable.rb:130:in `tap'
  origin (1.0.9) lib/origin/mergeable.rb:130:in `__multi__'
  origin (1.0.9) lib/origin/selectable.rb:382:in `or'
  cancan (1.6.8) lib/cancan/model_adapters/mongoid_adapter.rb:38:in `block in database_records'
  cancan (1.6.8) lib/cancan/model_adapters/mongoid_adapter.rb:36:in `each'
  cancan (1.6.8) lib/cancan/model_adapters/mongoid_adapter.rb:36:in `inject'
  cancan (1.6.8) lib/cancan/model_adapters/mongoid_adapter.rb:36:in `database_records'
  cancan (1.6.8) lib/cancan/model_additions.rb:23:in `accessible_by'
  cancan (1.6.8) lib/cancan/controller_resource.rb:81:in `load_collection'
  cancan (1.6.8) lib/cancan/controller_resource.rb:34:in `load_resource'
  cancan (1.6.8) lib/cancan/controller_resource.rb:25:in `load_and_authorize_resource'
  cancan (1.6.8) lib/cancan/controller_resource.rb:10:in `block in add_before_filter'
  activesupport (3.2.6) lib/active_support/callbacks.rb:429:in `_run__3279536705548101580__process_action__1827249083152788164__callbacks'
  activesupport (3.2.6) lib/active_support/callbacks.rb:405:in `__run_callback'
  activesupport (3.2.6) lib/active_support/callbacks.rb:385:in `_run_process_action_callbacks'
  activesupport (3.2.6) lib/active_support/callbacks.rb:81:in `run_callbacks'
  actionpack (3.2.6) lib/abstract_controller/callbacks.rb:17:in `process_action'
  actionpack (3.2.6) lib/action_controller/metal/rescue.rb:29:in `process_action'
  actionpack (3.2.6) lib/action_controller/metal/instrumentation.rb:30:in `block in process_action'
  activesupport (3.2.6) lib/active_support/notifications.rb:123:in `block in instrument'
  activesupport (3.2.6) lib/active_support/notifications/instrumenter.rb:20:in `instrument'
  activesupport (3.2.6) lib/active_support/notifications.rb:123:in `instrument'
  actionpack (3.2.6) lib/action_controller/metal/instrumentation.rb:29:in `process_action'
  actionpack (3.2.6) lib/action_controller/metal/params_wrapper.rb:206:in `process_action'
  actionpack (3.2.6) lib/abstract_controller/base.rb:121:in `process'
  actionpack (3.2.6) lib/abstract_controller/rendering.rb:45:in `process'
  actionpack (3.2.6) lib/action_controller/metal.rb:203:in `dispatch'
  actionpack (3.2.6) lib/action_controller/metal/rack_delegation.rb:14:in `dispatch'
  actionpack (3.2.6) lib/action_controller/metal.rb:246:in `block in action'
  actionpack (3.2.6) lib/action_dispatch/routing/route_set.rb:73:in `call'
  actionpack (3.2.6) lib/action_dispatch/routing/route_set.rb:73:in `dispatch'
  actionpack (3.2.6) lib/action_dispatch/routing/route_set.rb:36:in `call'
  journey (1.0.4) lib/journey/router.rb:68:in `block in call'
  journey (1.0.4) lib/journey/router.rb:56:in `each'
  journey (1.0.4) lib/journey/router.rb:56:in `call'
  actionpack (3.2.6) lib/action_dispatch/routing/route_set.rb:600:in `call'
  mongoid (3.0.9) lib/rack/mongoid/middleware/identity_map.rb:33:in `block in call'
  mongoid (3.0.9) lib/mongoid/unit_of_work.rb:39:in `unit_of_work'
  mongoid (3.0.9) lib/rack/mongoid/middleware/identity_map.rb:33:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/best_standards_support.rb:17:in `call'
  rack (1.4.1) lib/rack/etag.rb:23:in `call'
  rack (1.4.1) lib/rack/conditionalget.rb:25:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/head.rb:14:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/params_parser.rb:21:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/flash.rb:242:in `call'
  rack (1.4.1) lib/rack/session/abstract/id.rb:205:in `context'
  rack (1.4.1) lib/rack/session/abstract/id.rb:200:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/cookies.rb:338:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/callbacks.rb:28:in `block in call'
  activesupport (3.2.6) lib/active_support/callbacks.rb:405:in `_run__1299298641735140605__call__2567670475171850287__callbacks'
  activesupport (3.2.6) lib/active_support/callbacks.rb:405:in `__run_callback'
  activesupport (3.2.6) lib/active_support/callbacks.rb:385:in `_run_call_callbacks'
  activesupport (3.2.6) lib/active_support/callbacks.rb:81:in `run_callbacks'
  actionpack (3.2.6) lib/action_dispatch/middleware/callbacks.rb:27:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/reloader.rb:65:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/remote_ip.rb:31:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/debug_exceptions.rb:16:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/show_exceptions.rb:56:in `call'
  railties (3.2.6) lib/rails/rack/logger.rb:26:in `call_app'
  railties (3.2.6) lib/rails/rack/logger.rb:16:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/request_id.rb:22:in `call'
  rack (1.4.1) lib/rack/methodoverride.rb:21:in `call'
  rack (1.4.1) lib/rack/runtime.rb:17:in `call'
  activesupport (3.2.6) lib/active_support/cache/strategy/local_cache.rb:72:in `call'
  rack (1.4.1) lib/rack/lock.rb:15:in `call'
  actionpack (3.2.6) lib/action_dispatch/middleware/static.rb:62:in `call'
  railties (3.2.6) lib/rails/engine.rb:479:in `call'
  railties (3.2.6) lib/rails/application.rb:220:in `call'
  rack (1.4.1) lib/rack/content_length.rb:14:in `call'
  railties (3.2.6) lib/rails/rack/log_tailer.rb:17:in `call'
  rack (1.4.1) lib/rack/handler/webrick.rb:59:in `service'
  /Users/alan/.rvm/rubies/ruby-1.9.3-p194/lib/ruby/1.9.1/webrick/httpserver.rb:138:in `service'
  /Users/alan/.rvm/rubies/ruby-1.9.3-p194/lib/ruby/1.9.1/webrick/httpserver.rb:94:in `run'
  /Users/alan/.rvm/rubies/ruby-1.9.3-p194/lib/ruby/1.9.1/webrick/server.rb:191:in `block in start_thread'

  Rendered /Users/alan/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.6/lib/action_dispatch/middleware/templates/rescues/_trace.erb (2.5ms)
  Rendered /Users/alan/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.6/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb (0.9ms)
  Rendered /Users/alan/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.6/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb within rescues/layout (8.6ms)
al commented 11 years ago

Ah, sorry. More careful reading of the source tells me that this method should be receiving conditions as a Hash, not Mongoid::Criteria, so I guess this lays the blame on either CanCan or my usage of it.

I'll close this.

Some advice would be appreciated though. In this line from CanCan's Mongoid adapter, rule.conditions is a Mongoid::Criteria. Changing it to records.or rule.conditions.selector seems to solve my problem. Does that sound like the right thing to do from Origin's perspective?