Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Change in behavior with MongoDB projections and Mongoid 'only' method. #3533

Merged
merged 1 commit into from

6 participants

@rodrigosaito rodrigosaito From mongoid 3.1.x to mongoid 4.0.0 the behavior of 'only' method has…
… changed

You cannot access the id without explicit passing it as a parameter to 'only' method, changed to always have id as an attribute

fixes #3525
77e5875
@durran durran added this to the 4.0.0 milestone
@arthurnn
Owner

:+1: on my side..
@durran thoughts?

@arthurnn arthurnn merged commit bde8ac1 into from
@arthurnn
Owner

Thanks

@rodrigosaito rodrigosaito deleted the branch
@loopj

The use of selection.merge!({ "_id" => 1 }) from this PR breaks field exclusion, at least when using default_scope, for example:

class MyModel
  default_scope -> { without(:some_field) }
end

Doing any query on MyModel will now fail with a Moped::Errors::QueryFailure with the message Can't canonicalize query: BadValue Projection cannot have a mix of inclusion and exclusion.

This is because the generated query has both inclusive and exclusive rules in @fields:

@fields={"some_field"=>0, "_id"=>1}

Recommend reverting this PR until we find a safer fix for the original bug.

Any thoughts @arthurnn?

@arthurnn
Owner

@loopj Good point.. I will revert this for now, and write a regression test.
thanks for pointing it.

@arthurnn arthurnn referenced this pull request from a commit
@arthurnn arthurnn Revert "Merge pull request #3533 from rodrigosaito/include_id_in_only"
This breaks the following situation:
```
class MyModel
  default_scope -> { without(:some_field) }
end

MyModel.first
```

I will raise a Moped::Errors::QueryFailure, as a project cannot mix
inclusions and exclusions.

This reverts commit bde8ac1, reversing
changes made to c8af8d0.

Conflicts:
	lib/mongoid/attributes.rb
aa2c82e
@arthurnn
Owner

@rodrigosaito I had to revert this, because of what @loopj described.

@TomK32

I tracked this down as the source of the errors I get during my big upgrade.

ActiveModel::MissingAttributeError:
       Missing attribute: '_id'.

The selector is without the id Item.only(:_type) and using only quite a lot through the app I'd prefer not to change them all especially as the id is being loaded into the attributes.

 > i = Item.only(:item_ids, :_type).all[0]
#(Object doesn't support #inspect)
 > i.attributes
 #=> {"_id"=>BSON::ObjectId('538c1064d7f17875cd000435'), "_type"=>"SelectItem"}

The Object doesn't support #inspect seems to be a new error as well, what's going on here?

@fbernier

I am also experiencing this issue. Same with Object doesn't support #inspect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 18, 2014
  1. @rodrigosaito

    From mongoid 3.1.x to mongoid 4.0.0 the behavior of 'only' method has…

    rodrigosaito authored
    … changed
    
    You cannot access the id without explicit passing it as a parameter to 'only' method, changed to always have id as an attribute
    
    fixes #3525
This page is out of date. Refresh to see the latest.
Showing with 6 additions and 1 deletion.
  1. +2 −1  lib/mongoid/attributes.rb
  2. +4 −0 spec/mongoid/criteria_spec.rb
View
3  lib/mongoid/attributes.rb
@@ -224,7 +224,7 @@ def write_attributes(attrs = nil)
alias :attributes= :write_attributes
# Determine if the attribute is missing from the document, due to loading
- # it from the database with missing fields.
+ # it from the database with missing fields, _id is always loaded
#
# @example Is the attribute missing?
# document.attribute_missing?("test")
@@ -237,6 +237,7 @@ def write_attributes(attrs = nil)
def attribute_missing?(name)
selection = __selected_fields
return false unless selection
+ selection.merge!({ "_id" => 1 })
(selection.values.first == 0 && selection[name.to_s] == 0) ||
(selection.values.first == 1 && !selection.has_key?(name.to_s))
end
View
4 spec/mongoid/criteria_spec.rb
@@ -2640,6 +2640,10 @@ class D
it "does not add _type to the fields" do
expect(criteria.options[:fields]["_type"]).to be_nil
end
+
+ it "always contains the id field" do
+ expect(criteria.first.id).to_not be_nil
+ end
end
context "when instantiating a class of another type inside the iteration" do
Something went wrong with that request. Please try again.