From 32d1e68b1a61777d05ec98b1b0c11ab05cf704e4 Mon Sep 17 00:00:00 2001 From: Emily Date: Tue, 22 Dec 2015 17:06:21 +0100 Subject: [PATCH 1/6] MONGOID-4173 Add includable module for handling includes on a criteria --- lib/mongoid/criteria.rb | 12 +++++--- lib/mongoid/criteria/includable.rb | 43 ++++++++++++++++++++++++++++ spec/app/models/alert.rb | 1 + spec/app/models/post.rb | 1 + spec/mongoid/criteria_spec.rb | 46 ++++++++++++++++++++++++++---- 5 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 lib/mongoid/criteria/includable.rb diff --git a/lib/mongoid/criteria.rb b/lib/mongoid/criteria.rb index 43b8339d8b..86a264750b 100644 --- a/lib/mongoid/criteria.rb +++ b/lib/mongoid/criteria.rb @@ -1,5 +1,6 @@ # encoding: utf-8 require "mongoid/criteria/findable" +require "mongoid/criteria/includable" require "mongoid/criteria/inspectable" require "mongoid/criteria/marshalable" require "mongoid/criteria/modifiable" @@ -19,6 +20,7 @@ class Criteria include Origin::Queryable include Findable include Inspectable + include Includable include Marshalable include Modifiable include Scopable @@ -216,10 +218,12 @@ def initialize(klass) # # @since 2.2.0 def includes(*relations) - relations.flatten.each do |name| - metadata = klass.reflect_on_association(name) - raise Errors::InvalidIncludes.new(klass, relations) unless metadata - inclusions.push(metadata) unless inclusions.include?(metadata) + relations.flatten.each do |relation| + if relation.is_a?(Hash) + extract_nested_relation(klass, relation) + else + add_inclusion(klass, relation) + end end clone end diff --git a/lib/mongoid/criteria/includable.rb b/lib/mongoid/criteria/includable.rb new file mode 100644 index 0000000000..476edf1726 --- /dev/null +++ b/lib/mongoid/criteria/includable.rb @@ -0,0 +1,43 @@ +# encoding: utf-8 +module Mongoid + class Criteria + module Includable + + def add_inclusion(k, r) + metadata = get_inclusion_metadata(k, r) + raise Errors::InvalidIncludes.new(k, [ r ]) unless metadata + inclusions.push(metadata) unless inclusions.include?(metadata) + end + + def extract_relations_list(c, relations) + relations.each do |r| + if r.is_a?(Hash) + extract_nested_relation(c, r) + else + add_inclusion(c, r) + end + end + end + + def extract_nested_relation(c, relation) + relation.each do |k, v| + add_inclusion(c, k) + if v.is_a?(Array) + extract_relations_list(k, v) + else + add_inclusion(k, v) + end + end + end + + def get_inclusion_metadata(k, n) + if k.is_a?(Class) + k.reflect_on_association(n) + else + k = k.to_s.classify.constantize + k.reflect_on_association(n) + end + end + end + end +end diff --git a/spec/app/models/alert.rb b/spec/app/models/alert.rb index 48ed2eb402..89a29b18d1 100644 --- a/spec/app/models/alert.rb +++ b/spec/app/models/alert.rb @@ -2,4 +2,5 @@ class Alert include Mongoid::Document field :message, type: String belongs_to :account + has_many :items end diff --git a/spec/app/models/post.rb b/spec/app/models/post.rb index 4a8b41cb06..13bf317f2f 100644 --- a/spec/app/models/post.rb +++ b/spec/app/models/post.rb @@ -14,6 +14,7 @@ class Post has_and_belongs_to_many :tags, before_add: :before_add_tag, after_add: :after_add_tag, before_remove: :before_remove_tag, after_remove: :after_remove_tag has_many :videos, validate: false has_many :roles, validate: false + has_many :alerts belongs_to :posteable, polymorphic: true accepts_nested_attributes_for :posteable, autosave: true diff --git a/spec/mongoid/criteria_spec.rb b/spec/mongoid/criteria_spec.rb index 19fc523dde..e7c217ca07 100644 --- a/spec/mongoid/criteria_spec.rb +++ b/spec/mongoid/criteria_spec.rb @@ -1147,12 +1147,48 @@ end end - context "when providing a hash" do + context "when providing a list of associations" do - it "raises an error" do - expect { - Person.includes(preferences: :members) - }.to raise_error(Mongoid::Errors::InvalidIncludes) + let!(:user) do + User.create + end + + let(:results) do + User.includes(:posts, :descriptions).to_a + end + + it "executes the query" do + expect(results.first).to eq(user) + end + end + + context "when providing a nested association" do + + let!(:user) do + User.create + end + + let(:results) do + User.includes(:posts => [:alerts]).to_a + end + + it "executes the query" do + expect(results.first).to eq(user) + end + end + + context "when providing a deeply nested association" do + + let!(:user) do + User.create + end + + let(:results) do + User.includes(:posts => [{ :alerts => :items }]).to_a + end + + it "executes the query" do + expect(results.first).to eq(user) end end From 7e5518ba857f901917b25caec22cdba7371fa4fc Mon Sep 17 00:00:00 2001 From: Emily Date: Wed, 23 Dec 2015 15:07:09 +0100 Subject: [PATCH 2/6] MONGOID-4173 Group inclusions by class and relation --- lib/mongoid/criteria/includable.rb | 3 +-- lib/mongoid/relations/eager.rb | 7 +++---- lib/mongoid/relations/eager/base.rb | 4 +++- spec/app/models/alert.rb | 1 + spec/mongoid/criteria_spec.rb | 11 +++++++++++ 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/mongoid/criteria/includable.rb b/lib/mongoid/criteria/includable.rb index 476edf1726..913a72c0cf 100644 --- a/lib/mongoid/criteria/includable.rb +++ b/lib/mongoid/criteria/includable.rb @@ -34,8 +34,7 @@ def get_inclusion_metadata(k, n) if k.is_a?(Class) k.reflect_on_association(n) else - k = k.to_s.classify.constantize - k.reflect_on_association(n) + k.to_s.classify.constantize.reflect_on_association(n) end end end diff --git a/lib/mongoid/relations/eager.rb b/lib/mongoid/relations/eager.rb index 779a2fb01e..ffd995e997 100644 --- a/lib/mongoid/relations/eager.rb +++ b/lib/mongoid/relations/eager.rb @@ -34,11 +34,10 @@ def eager_load(docs) end def preload(relations, docs) - relations.group_by do |metadata| - metadata.relation - end.each do |relation, associations| - relation.eager_load_klass.new(associations, docs).run + { relation: metadata.relation, klass: metadata.inverse_class_name } + end.each do |relation_class, associations| + docs = relation_class[:relation].eager_load_klass.new(associations, docs).run end end end diff --git a/lib/mongoid/relations/eager/base.rb b/lib/mongoid/relations/eager/base.rb index 8d9da20003..0a19601eff 100644 --- a/lib/mongoid/relations/eager/base.rb +++ b/lib/mongoid/relations/eager/base.rb @@ -45,10 +45,12 @@ def shift_metadata # # @since 4.0.0 def run + @loaded = [] while shift_metadata preload + @loaded << @docs.collect { |d| d.send(@metadata.name) } end - @docs + @loaded.flatten end # Preload the current relation. diff --git a/spec/app/models/alert.rb b/spec/app/models/alert.rb index 89a29b18d1..76976c615a 100644 --- a/spec/app/models/alert.rb +++ b/spec/app/models/alert.rb @@ -3,4 +3,5 @@ class Alert field :message, type: String belongs_to :account has_many :items + belongs_to :post end diff --git a/spec/mongoid/criteria_spec.rb b/spec/mongoid/criteria_spec.rb index e7c217ca07..2e5dafe90c 100644 --- a/spec/mongoid/criteria_spec.rb +++ b/spec/mongoid/criteria_spec.rb @@ -1168,6 +1168,17 @@ User.create end + before do + p = Post.new + a = Alert.new + p.alerts << a + a.save + p.save + user.posts << p + p.save + user.save + end + let(:results) do User.includes(:posts => [:alerts]).to_a end From ca3e7cfde82e13dfa2fb1d3c31c21d5444a7ccd2 Mon Sep 17 00:00:00 2001 From: Emily Date: Wed, 23 Dec 2015 16:45:36 +0100 Subject: [PATCH 3/6] MONGOID-4173 Group inclusion definitions by class name, then by relation --- lib/mongoid/criteria.rb | 2 +- lib/mongoid/criteria/includable.rb | 60 +++++++++++++++++++++++++----- lib/mongoid/relations/eager.rb | 16 ++++++-- spec/mongoid/criteria_spec.rb | 2 - 4 files changed, 63 insertions(+), 17 deletions(-) diff --git a/lib/mongoid/criteria.rb b/lib/mongoid/criteria.rb index 86a264750b..57d5881ed1 100644 --- a/lib/mongoid/criteria.rb +++ b/lib/mongoid/criteria.rb @@ -220,7 +220,7 @@ def initialize(klass) def includes(*relations) relations.flatten.each do |relation| if relation.is_a?(Hash) - extract_nested_relation(klass, relation) + extract_nested_inclusion(klass, relation) else add_inclusion(klass, relation) end diff --git a/lib/mongoid/criteria/includable.rb b/lib/mongoid/criteria/includable.rb index 913a72c0cf..e75692c945 100644 --- a/lib/mongoid/criteria/includable.rb +++ b/lib/mongoid/criteria/includable.rb @@ -1,25 +1,56 @@ # encoding: utf-8 module Mongoid class Criteria + + # Module providing functionality for parsing (nested) inclusion definitions. module Includable - def add_inclusion(k, r) - metadata = get_inclusion_metadata(k, r) - raise Errors::InvalidIncludes.new(k, [ r ]) unless metadata + # Add an inclusion definition to the list of inclusions for the criteria. + # + # @example Add an inclusion. + # criteria.add_inclusion(Person, :posts) + # + # @param [ Class, String, Symbol ] k The class or string/symbol of the class name. + # @param [ Symbol ] relation The relation. + # + # @raise [ Errors::InvalidIncludes ] If no relation is found. + # + # @since 5.1.0 + def add_inclusion(k, relation) + metadata = get_inclusion_metadata(k, relation) + raise Errors::InvalidIncludes.new(k, [ relation ]) unless metadata inclusions.push(metadata) unless inclusions.include?(metadata) end - def extract_relations_list(c, relations) + # Extract inclusion definitions from a list. + # + # @example Extract the inclusions from a list. + # criteria.extract_relations_list(:posts, [{ :alerts => :items }]) + # + # @param [ Symbol ] association The name of the association. + # @param [ Array, Symbol ] relations Either the relation name or a list of associations. + # + # @since 5.1.0 + def extract_relations_list(association, relations) relations.each do |r| if r.is_a?(Hash) - extract_nested_relation(c, r) + extract_nested_inclusion(association, r) else - add_inclusion(c, r) + add_inclusion(association, r) end end end - def extract_nested_relation(c, relation) + # Extract nested inclusion. + # + # @example Extract the inclusions from a nested definition. + # criteria.extract_nested_inclusion(User, { :posts => [:alerts] }) + # + # @param [ Class, Symbol ] c The class for which the inclusion should be added. + # @param [ Hash ] relation The nested inclusion. + # + # @since 5.1.0 + def extract_nested_inclusion(c, relation) relation.each do |k, v| add_inclusion(c, k) if v.is_a?(Array) @@ -30,11 +61,20 @@ def extract_nested_relation(c, relation) end end - def get_inclusion_metadata(k, n) + # Get the metadata for an inclusion. + # + # @example Get the metadata for an inclusion definition. + # criteria.get_inclusion_metadata(User, :posts) + # + # @param [ Class, Symbol, String ] k The class for determining the association metadata + # @param [ Symbol ] association The name of the association. + # + # @since 5.1.0 + def get_inclusion_metadata(k, association) if k.is_a?(Class) - k.reflect_on_association(n) + k.reflect_on_association(association) else - k.to_s.classify.constantize.reflect_on_association(n) + k.to_s.classify.constantize.reflect_on_association(association) end end end diff --git a/lib/mongoid/relations/eager.rb b/lib/mongoid/relations/eager.rb index ffd995e997..a8a9c4a19b 100644 --- a/lib/mongoid/relations/eager.rb +++ b/lib/mongoid/relations/eager.rb @@ -34,10 +34,18 @@ def eager_load(docs) end def preload(relations, docs) - relations.group_by do |metadata| - { relation: metadata.relation, klass: metadata.inverse_class_name } - end.each do |relation_class, associations| - docs = relation_class[:relation].eager_load_klass.new(associations, docs).run + rs = relations.group_by do |metadata| + metadata.inverse_class_name + end + rs.keys.each do |k| + rs[k] = rs[k].group_by do |metadata| + metadata.relation + end + end + rs.each do |k, associations| + docs = associations.collect do |r, a| + r.eager_load_klass.new(a, docs).run + end.flatten end end end diff --git a/spec/mongoid/criteria_spec.rb b/spec/mongoid/criteria_spec.rb index 2e5dafe90c..afa75f7e63 100644 --- a/spec/mongoid/criteria_spec.rb +++ b/spec/mongoid/criteria_spec.rb @@ -1173,9 +1173,7 @@ a = Alert.new p.alerts << a a.save - p.save user.posts << p - p.save user.save end From c60c5e1cfff64ae4d5505a461a96a028fd884847 Mon Sep 17 00:00:00 2001 From: Emily Date: Wed, 6 Jan 2016 17:20:38 +0100 Subject: [PATCH 4/6] MONGOID-4173 Clearer variable names and test improvements --- lib/mongoid/criteria.rb | 58 ----------------- lib/mongoid/criteria/includable.rb | 100 +++++++++++++++++++++++------ spec/mongoid/criteria_spec.rb | 39 +++++++---- 3 files changed, 107 insertions(+), 90 deletions(-) diff --git a/lib/mongoid/criteria.rb b/lib/mongoid/criteria.rb index 57d5881ed1..f607df5933 100644 --- a/lib/mongoid/criteria.rb +++ b/lib/mongoid/criteria.rb @@ -196,64 +196,6 @@ def initialize(klass) klass ? super(klass.aliased_fields, klass.fields) : super({}, {}) end - # Eager loads all the provided relations. Will load all the documents - # into the identity map whose ids match based on the extra query for the - # ids. - # - # @note This will work for embedded relations that reference another - # collection via belongs_to as well. - # - # @note Eager loading brings all the documents into memory, so there is a - # sweet spot on the performance gains. Internal benchmarks show that - # eager loading becomes slower around 100k documents, but this will - # naturally depend on the specific application. - # - # @example Eager load the provided relations. - # Person.includes(:posts, :game) - # - # @param [ Array ] relations The names of the relations to eager - # load. - # - # @return [ Criteria ] The cloned criteria. - # - # @since 2.2.0 - def includes(*relations) - relations.flatten.each do |relation| - if relation.is_a?(Hash) - extract_nested_inclusion(klass, relation) - else - add_inclusion(klass, relation) - end - end - clone - end - - # Get a list of criteria that are to be executed for eager loading. - # - # @example Get the eager loading inclusions. - # Person.includes(:game).inclusions - # - # @return [ Array ] The inclusions. - # - # @since 2.2.0 - def inclusions - @inclusions ||= [] - end - - # Set the inclusions for the criteria. - # - # @example Set the inclusions. - # criteria.inclusions = [ meta ] - # - # @param [ Array ] The inclusions. - # - # @return [ Array ] The new inclusions. - # - # @since 3.0.0 - def inclusions=(value) - @inclusions = value - end - # Merges another object with this +Criteria+ and returns a new criteria. # The other object may be a +Criteria+ or a +Hash+. This is used to # combine multiple scopes together, where a chained scope situation diff --git a/lib/mongoid/criteria/includable.rb b/lib/mongoid/criteria/includable.rb index e75692c945..534d2d352e 100644 --- a/lib/mongoid/criteria/includable.rb +++ b/lib/mongoid/criteria/includable.rb @@ -5,20 +5,80 @@ class Criteria # Module providing functionality for parsing (nested) inclusion definitions. module Includable + # Eager loads all the provided relations. Will load all the documents + # into the identity map whose ids match based on the extra query for the + # ids. + # + # @note This will work for embedded relations that reference another + # collection via belongs_to as well. + # + # @note Eager loading brings all the documents into memory, so there is a + # sweet spot on the performance gains. Internal benchmarks show that + # eager loading becomes slower around 100k documents, but this will + # naturally depend on the specific application. + # + # @example Eager load the provided relations. + # Person.includes(:posts, :game) + # + # @param [ Array, Array ] relations The names of the relations to eager + # load. + # + # @return [ Criteria ] The cloned criteria. + # + # @since 2.2.0 + def includes(*relations) + relations.flatten.each do |relation| + if relation.is_a?(Hash) + extract_nested_inclusion(klass, relation) + else + add_inclusion(klass, relation) + end + end + clone + end + + # Get a list of criteria that are to be executed for eager loading. + # + # @example Get the eager loading inclusions. + # Person.includes(:game).inclusions + # + # @return [ Array ] The inclusions. + # + # @since 2.2.0 + def inclusions + @inclusions ||= [] + end + + # Set the inclusions for the criteria. + # + # @example Set the inclusions. + # criteria.inclusions = [ meta ] + # + # @param [ Array ] The inclusions. + # + # @return [ Array ] The new inclusions. + # + # @since 3.0.0 + def inclusions=(value) + @inclusions = value + end + + private + # Add an inclusion definition to the list of inclusions for the criteria. # # @example Add an inclusion. # criteria.add_inclusion(Person, :posts) # - # @param [ Class, String, Symbol ] k The class or string/symbol of the class name. + # @param [ Class, String, Symbol ] _klass The class or string/symbol of the class name. # @param [ Symbol ] relation The relation. # # @raise [ Errors::InvalidIncludes ] If no relation is found. # # @since 5.1.0 - def add_inclusion(k, relation) - metadata = get_inclusion_metadata(k, relation) - raise Errors::InvalidIncludes.new(k, [ relation ]) unless metadata + def add_inclusion(_klass, relation) + metadata = get_inclusion_metadata(_klass, relation) + raise Errors::InvalidIncludes.new(_klass, [ relation ]) unless metadata inclusions.push(metadata) unless inclusions.include?(metadata) end @@ -32,11 +92,11 @@ def add_inclusion(k, relation) # # @since 5.1.0 def extract_relations_list(association, relations) - relations.each do |r| - if r.is_a?(Hash) - extract_nested_inclusion(association, r) + relations.each do |relation| + if relation.is_a?(Hash) + extract_nested_inclusion(association, relation) else - add_inclusion(association, r) + add_inclusion(association, relation) end end end @@ -46,17 +106,17 @@ def extract_relations_list(association, relations) # @example Extract the inclusions from a nested definition. # criteria.extract_nested_inclusion(User, { :posts => [:alerts] }) # - # @param [ Class, Symbol ] c The class for which the inclusion should be added. + # @param [ Class, Symbol ] _klass The class for which the inclusion should be added. # @param [ Hash ] relation The nested inclusion. # # @since 5.1.0 - def extract_nested_inclusion(c, relation) - relation.each do |k, v| - add_inclusion(c, k) - if v.is_a?(Array) - extract_relations_list(k, v) + def extract_nested_inclusion(_klass, relation) + relation.each do |association, _inclusion| + add_inclusion(_klass, association) + if _inclusion.is_a?(Array) + extract_relations_list(association, _inclusion) else - add_inclusion(k, v) + add_inclusion(association, _inclusion) end end end @@ -66,15 +126,15 @@ def extract_nested_inclusion(c, relation) # @example Get the metadata for an inclusion definition. # criteria.get_inclusion_metadata(User, :posts) # - # @param [ Class, Symbol, String ] k The class for determining the association metadata + # @param [ Class, Symbol, String ] _klass The class for determining the association metadata # @param [ Symbol ] association The name of the association. # # @since 5.1.0 - def get_inclusion_metadata(k, association) - if k.is_a?(Class) - k.reflect_on_association(association) + def get_inclusion_metadata(_klass, association) + if _klass.is_a?(Class) + _klass.reflect_on_association(association) else - k.to_s.classify.constantize.reflect_on_association(association) + _klass.to_s.classify.constantize.reflect_on_association(association) end end end diff --git a/spec/mongoid/criteria_spec.rb b/spec/mongoid/criteria_spec.rb index afa75f7e63..ccfc41c97a 100644 --- a/spec/mongoid/criteria_spec.rb +++ b/spec/mongoid/criteria_spec.rb @@ -1150,15 +1150,28 @@ context "when providing a list of associations" do let!(:user) do - User.create + User.create(posts: [ post1 ], descriptions: [ description1 ]) end - let(:results) do - User.includes(:posts, :descriptions).to_a + let!(:post1) do + Post.create + end + + let!(:description1) do + Description.create(details: 1) + end + + let(:result) do + User.includes(:posts, :descriptions).first end it "executes the query" do - expect(results.first).to eq(user) + expect(result).to eq(user) + end + + it "includes the related objects" do + expect(result.posts).to eq([ post1 ]) + expect(result.descriptions).to eq([ description1 ]) end end @@ -1169,20 +1182,22 @@ end before do - p = Post.new - a = Alert.new - p.alerts << a - a.save - user.posts << p + p = Post.create(alerts: [ Alert.create ]) + user.posts = [ p ] user.save end - let(:results) do - User.includes(:posts => [:alerts]).to_a + let(:result) do + User.includes(:posts => [:alerts]).first end it "executes the query" do - expect(results.first).to eq(user) + expect(result).to eq(user) + end + + it "includes the related objects" do + expect(result.posts.size).to eq(1) + expect(result.posts.first.alerts.size).to eq(1) end end From 1bd0698895c7a96368a9ace7f629d85e4f4a62e6 Mon Sep 17 00:00:00 2001 From: Emily Date: Thu, 7 Jan 2016 11:37:01 +0100 Subject: [PATCH 5/6] MONGOID-4173 Clearer variable names --- lib/mongoid/relations/eager.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/mongoid/relations/eager.rb b/lib/mongoid/relations/eager.rb index a8a9c4a19b..a7952b2b58 100644 --- a/lib/mongoid/relations/eager.rb +++ b/lib/mongoid/relations/eager.rb @@ -34,17 +34,17 @@ def eager_load(docs) end def preload(relations, docs) - rs = relations.group_by do |metadata| + grouped_relations = relations.group_by do |metadata| metadata.inverse_class_name end - rs.keys.each do |k| - rs[k] = rs[k].group_by do |metadata| + grouped_relations.keys.each do |_klass| + grouped_relations[_klass] = grouped_relations[_klass].group_by do |metadata| metadata.relation end end - rs.each do |k, associations| - docs = associations.collect do |r, a| - r.eager_load_klass.new(a, docs).run + grouped_relations.each do |_klass, associations| + docs = associations.collect do |_relation, association| + _relation.eager_load_klass.new(association, docs).run end.flatten end end From a62d525cee25e395bdcec2c5f781c8c61c9bc4f8 Mon Sep 17 00:00:00 2001 From: Emily Date: Fri, 22 Jan 2016 13:05:43 +0100 Subject: [PATCH 6/6] MONGOID-4173 Update method documentation --- lib/mongoid/criteria/includable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mongoid/criteria/includable.rb b/lib/mongoid/criteria/includable.rb index 534d2d352e..08edbfdbb1 100644 --- a/lib/mongoid/criteria/includable.rb +++ b/lib/mongoid/criteria/includable.rb @@ -88,7 +88,7 @@ def add_inclusion(_klass, relation) # criteria.extract_relations_list(:posts, [{ :alerts => :items }]) # # @param [ Symbol ] association The name of the association. - # @param [ Array, Symbol ] relations Either the relation name or a list of associations. + # @param [ Array ] relations A list of associations. # # @since 5.1.0 def extract_relations_list(association, relations)