Skip to content

Commit

Permalink
Eager loading now supported on many to many relations. Closes #1348
Browse files Browse the repository at this point in the history
  • Loading branch information
durran committed Jan 16, 2012
1 parent c72fee8 commit d4caf38
Show file tree
Hide file tree
Showing 10 changed files with 268 additions and 32 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ For instructions on upgrading to newer versions, visit

### New Features

* \#1348 Eager loading is now supported on many-to-many relations.

### Major Changes

* `Model.defaults` no longer exists. You may get all defaults with a
Expand Down
6 changes: 3 additions & 3 deletions lib/mongoid/contexts/mongo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,12 @@ def distinct(field)
#
# @since 2.4.1
def eager_load(docs)
parent_ids = docs.map(&:id)
criteria.inclusions.reject! do |metadata|
if metadata.macro == :belongs_to
child_ids = load_ids(metadata.foreign_key)
if metadata.stores_foreign_key?
child_ids = load_ids(metadata.foreign_key).flatten
metadata.eager_load(child_ids)
else
parent_ids = docs.map(&:id)
metadata.eager_load(parent_ids)
end
end
Expand Down
9 changes: 8 additions & 1 deletion lib/mongoid/identity_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,14 @@ class IdentityMap < Hash
# @since 2.1.0
def get(klass, identifier)
return nil unless Mongoid.identity_map_enabled? && klass
documents_for(klass)[identifier]
if identifier.is_a?(::Array)
documents = documents_for(klass)
identifier.map do |id|
documents[id] || (return nil)
end
else
documents_for(klass)[identifier]
end
end

# Remove the document from the identity map.
Expand Down
4 changes: 3 additions & 1 deletion lib/mongoid/relations/builders/referenced/many_to_many.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ class ManyToMany < Builder
# @return [ Array<Document> ] The documents.
def build(type = nil)
return object.try(:dup) unless query?
metadata.criteria(object || [])
ids = object || []
crit = metadata.criteria(ids)
IdentityMap.get(crit.klass, ids) || crit
end

# Do we need to perform a database query? It will be so if the object we
Expand Down
8 changes: 5 additions & 3 deletions lib/mongoid/relations/referenced/many_to_many.rb
Original file line number Diff line number Diff line change
Expand Up @@ -244,13 +244,15 @@ def criteria(metadata, object, type = nil)
# Proxy.eager_load(metadata, criteria)
#
# @param [ Metadata ] metadata The relation metadata.
# @param [ Criteria ] criteria The criteria being used.
# @param [ Array<Object> ] ids The ids of the documents to load.
#
# @return [ Criteria ] The criteria to eager load the relation.
#
# @since 2.2.0
def eager_load(metadata, criteria)
raise Errors::EagerLoad.new(metadata.name)
def eager_load(metadata, ids)
metadata.klass.any_in(:_id => ids).each do |doc|
IdentityMap.set(doc)
end
end

# Returns true if the relation is an embedded one. In this case
Expand Down
24 changes: 23 additions & 1 deletion perf/benchmark.rb
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,15 @@
end
end

[ 1000, 10000].each do |i|
[ 1000, 10000 ].each do |i|

GC.start

i.times do |n|

Person.create(:title => "#{n}").tap do |person|
person.posts.create(:title => "#{n}")
person.preferences.create(:name => "#{n}")
end
end

Expand Down Expand Up @@ -352,5 +353,26 @@

Mongoid.identity_map_enabled = false
end

puts "\n[ Iterate with association load n-n ]"

Mongoid.unit_of_work do

bm.report("#each [ normal ] ") do
Person.all.each do |person|
person.preferences.each { |preference| preference.name }
end
end

Mongoid.identity_map_enabled = true

bm.report("#each [ eager ] ") do
Person.includes(:preferences).each do |person|
person.preferences.each { |preference| preference.name }
end
end

Mongoid.identity_map_enabled = false
end
end
end
131 changes: 131 additions & 0 deletions spec/functional/mongoid/criterion/inclusion_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,137 @@
Person.create(:ssn => "123-12-1211")
end

context "when including a has and belongs to many" do

let!(:preference_one) do
person.preferences.create(:name => "one")
end

let!(:preference_two) do
person.preferences.create(:name => "two")
end

context "when the criteria has no options" do

before do
Mongoid::IdentityMap.clear
end

let!(:criteria) do
Person.includes(:preferences).entries
end

it "returns the correct documents" do
criteria.should eq([ person ])
end

let(:preference_map) do
Mongoid::IdentityMap[Preference.collection_name]
end

it "inserts the first document into the identity map" do
preference_map[preference_one.id].should eq(preference_one)
end

it "inserts the second document into the identity map" do
preference_map[preference_two.id].should eq(preference_two)
end
end

context "when calling first on the criteria" do

before do
Mongoid::IdentityMap.clear
end

let!(:from_db) do
Person.includes(:preferences).first
end

it "returns the correct documents" do
from_db.should eq(person)
end

let(:preference_map) do
Mongoid::IdentityMap[Preference.collection_name]
end

it "inserts the first document into the identity map" do
preference_map[preference_one.id].should eq(preference_one)
end

it "inserts the second document into the identity map" do
preference_map[preference_two.id].should eq(preference_two)
end
end

context "when calling last on the criteria" do

before do
Mongoid::IdentityMap.clear
end

let!(:from_db) do
Person.includes(:preferences).last
end

it "returns the correct documents" do
from_db.should eq(person)
end

let(:preference_map) do
Mongoid::IdentityMap[Preference.collection_name]
end

it "inserts the first document into the identity map" do
preference_map[preference_one.id].should eq(preference_one)
end

it "inserts the second document into the identity map" do
preference_map[preference_two.id].should eq(preference_two)
end
end

context "when the criteria has limiting options" do

let!(:person_two) do
Person.create(:ssn => "123-43-2123")
end

let!(:preference_three) do
person_two.preferences.create(:name => "three")
end

before do
Mongoid::IdentityMap.clear
end

let!(:criteria) do
Person.includes(:preferences).asc(:_id).limit(1).entries
end

let(:preference_map) do
Mongoid::IdentityMap[Preference.collection_name]
end

it "returns the correct documents" do
criteria.should eq([ person ])
end

it "inserts the first document into the identity map" do
preference_map[preference_one.id].should eq(preference_one)
end

it "inserts the second document into the identity map" do
preference_map[preference_two.id].should eq(preference_two)
end

it "does not insert the third preference into the identity map" do
preference_map[preference_three.id].should be_nil
end
end
end

context "when including a has many" do

let!(:post_one) do
Expand Down
45 changes: 39 additions & 6 deletions spec/functional/mongoid/relations/referenced/many_to_many_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1504,14 +1504,47 @@

describe ".eager_load" do

let(:metadata) do
Person.relations["preferences"]
before do
Mongoid.identity_map_enabled = true
end

it "raises an error" do
expect {
described_class.eager_load(metadata, Person.all)
}.to raise_error(Mongoid::Errors::EagerLoad)
after do
Mongoid.identity_map_enabled = false
end

context "when the relation is not polymorphic" do

let!(:person) do
Person.create(:ssn => "243-12-5243")
end

let!(:preference) do
person.preferences.create(:name => "testing")
end

let(:metadata) do
Person.relations["preferences"]
end

let(:ids) do
[ preference.id ]
end

let!(:eager) do
described_class.eager_load(metadata, ids)
end

let(:map) do
Mongoid::IdentityMap.get(Preference, ids)
end

it "returns the appropriate criteria" do
eager.selector.should eq({ :_id => { "$in" => ids }})
end

it "puts the documents in the identity map" do
map.should eq([ preference ])
end
end
end

Expand Down
33 changes: 33 additions & 0 deletions spec/unit/mongoid/identity_map_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,39 @@
end
end

context "when getting by an array of ids" do

context "when the document exists in the identity map" do

before do
identity_map.set(person)
end

let(:get) do
identity_map.get(Person, [ person.id ])
end

it "returns the matching documents" do
get.should eq([ person ])
end
end

context "when any id is not found in the map" do

before do
identity_map.set(person)
end

let(:get) do
identity_map.get(Person, [ person.id, BSON::ObjectId.new ])
end

it "returns nil" do
get.should be_nil
end
end
end

context "when getting by selector" do

let!(:post_one) do
Expand Down
Loading

0 comments on commit d4caf38

Please sign in to comment.