Skip to content

Commit 083ecfb

Browse files
authored
MONGOID-2586 cannot access parent object in the after_find function (#5174)
* MONGOID-2586 add test * MONGOID-2586 potential fix * MONGOID-2586 introduce pending callbacks * MONGOID-2586 remove block from instantiate * MONGOID-2586 remove yield and add after initialize test * MONGOID-2586 fix session tests * MONGOID-2586 pending test for CI * MONGOID-2586 fix interceptable tests * MONGOID-2586 exclude hasmany enumerables * MONGOID-2586 change condition * MONGOID-2586 only execute callbacks on embedded associations * MONGOID-2586 change callbacks option * MONGOID-2586 move tests and add callback * MONGOID-2586 change default on defer_callbacks * MONGOID-2586 add tests for other associations * MONGOID-2586 add embed_one tests * MONGOID-2586 update comments and remove TODOs * MONGOID-2586 delay defaults and refactor tests * MONGOID-2586 update the comments * MONGOID-2586 update comments * MONGOID-2586 clear pending callbacks * MONGOID-2586 defer callbacks for unpersisted intializers. Add tests for HABTM and create/build methods * MONGOID-2586 change param to avoid attribute failures * MONGOID-2586 change instantiate params * MONGOID-2586 change params for instantiate * MONGOID-2586 change params to avoid errors * MONGOID-2586 more param related errors * MONGOID-2586 remove byebug * MONGOID-2586 extract the constructor * MONGOID-2586 remove splat operator * MONGOID-2586 add private api and comments * MONGOID-2586 duplicate factory functions * MONGOID-2586 duplicate instantiate, change to keyword argument * MONGOID-2586 add back block functionality to instantiate * MONGOID-2586 flip and rename the flag * MONGOID-2586 remove byebugs * MONGOID-2586 update comments * MONGOID-2586 change comment
1 parent 479216a commit 083ecfb

File tree

20 files changed

+541
-55
lines changed

20 files changed

+541
-55
lines changed

lib/mongoid/association/accessors.rb

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,23 @@ def __build__(name, object, association, selected_fields = nil)
4242
# @return [ Proxy ] The association.
4343
def create_relation(object, association, selected_fields = nil)
4444
type = @attributes[association.inverse_type]
45-
target = association.build(self, object, type, selected_fields)
46-
target ? association.create_relation(self, target) : nil
45+
target = if t = association.build(self, object, type, selected_fields)
46+
association.create_relation(self, t)
47+
else
48+
nil
49+
end
50+
51+
# Only need to do this on embedded associations. The pending callbacks
52+
# are only added when materializing the documents, which only happens
53+
# on embedded associations. There is no call to the database in the
54+
# construction of a referenced association.
55+
if association.embedded?
56+
Array(target).each do |doc|
57+
doc.try(:run_pending_callbacks)
58+
end
59+
end
60+
61+
target
4762
end
4863

4964
# Resets the criteria inside the association proxy. Used by many-to-many

lib/mongoid/association/builders.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,10 @@ def self.define_builder!(association)
4646
association.inverse_class.tap do |klass|
4747
klass.re_define_method("build_#{association.name}") do |*args|
4848
attributes, _options = parse_args(*args)
49-
document = Factory.build(association.relation_class, attributes)
49+
document = Factory.execute_build(association.relation_class, attributes, execute_callbacks: false)
5050
_building do
5151
child = send("#{association.name}=", document)
52+
child.run_pending_callbacks
5253
child.run_callbacks(:build)
5354
child
5455
end
@@ -69,10 +70,11 @@ def self.define_creator!(association)
6970
association.inverse_class.tap do |klass|
7071
klass.re_define_method("create_#{association.name}") do |*args|
7172
attributes, _options = parse_args(*args)
72-
document = Factory.build(association.relation_class, attributes)
73+
document = Factory.execute_build(association.relation_class, attributes, execute_callbacks: false)
7374
doc = _assigning do
7475
send("#{association.name}=", document)
7576
end
77+
doc.run_pending_callbacks
7678
doc.save
7779
save if new_record? && association.stores_foreign_key?
7880
doc

lib/mongoid/association/embedded/embeds_many/buildable.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def build(base, object, type = nil, selected_fields = nil)
3131
docs = []
3232
object.each do |attrs|
3333
if _loading? && base.persisted?
34-
docs.push(Factory.from_db(klass, attrs, nil, selected_fields))
34+
docs.push(Factory.execute_from_db(klass, attrs, nil, selected_fields, execute_callbacks: false))
3535
else
3636
docs.push(Factory.build(klass, attrs))
3737
end

lib/mongoid/association/embedded/embeds_many/proxy.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,11 @@ def concat(docs)
6767
#
6868
# @return [ Document ] The new document.
6969
def build(attributes = {}, type = nil)
70-
doc = Factory.build(type || _association.klass, attributes)
70+
doc = Factory.execute_build(type || _association.klass, attributes, execute_callbacks: false)
7171
append(doc)
7272
doc.apply_post_processed_defaults
7373
yield(doc) if block_given?
74+
doc.run_pending_callbacks
7475
doc.run_callbacks(:build) { doc }
7576
_base._reset_memoized_descendants!
7677
doc

lib/mongoid/association/embedded/embeds_one/buildable.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ module Buildable
2727
def build(base, object, _type = nil, selected_fields = nil)
2828
return object unless object.is_a?(Hash)
2929
if _loading? && base.persisted?
30-
Factory.from_db(klass, object, nil, selected_fields)
30+
Factory.execute_from_db(klass, object, nil, selected_fields, execute_callbacks: false)
3131
else
3232
Factory.build(klass, object)
3333
end

lib/mongoid/association/many.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ def create!(attributes = nil, type = nil, &block)
6161
attributes.map { |attrs| create!(attrs, type, &block) }
6262
else
6363
doc = build(attributes, type, &block)
64+
65+
Array(doc).each do |doc|
66+
doc.try(:run_pending_callbacks)
67+
end
68+
6469
_base.persisted? ? doc.save! : raise_unsaved(doc)
6570
doc
6671
end

lib/mongoid/association/referenced/has_and_belongs_to_many/proxy.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,13 @@ def concat(documents)
8383
#
8484
# @return [ Document ] The new document.
8585
def build(attributes = {}, type = nil)
86-
doc = Factory.build(type || klass, attributes)
86+
doc = Factory.execute_build(type || klass, attributes, execute_callbacks: false)
8787
_base.public_send(foreign_key).push(doc.public_send(_association.primary_key))
8888
append(doc)
8989
doc.apply_post_processed_defaults
9090
unsynced(doc, inverse_foreign_key)
9191
yield(doc) if block_given?
92+
doc.run_pending_callbacks
9293
doc
9394
end
9495

lib/mongoid/association/referenced/has_many/proxy.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,11 @@ def concat(documents)
7171
#
7272
# @return [ Document ] The new document.
7373
def build(attributes = {}, type = nil)
74-
doc = Factory.build(type || klass, attributes)
74+
doc = Factory.execute_build(type || klass, attributes, execute_callbacks: false)
7575
append(doc)
7676
doc.apply_post_processed_defaults
7777
yield(doc) if block_given?
78+
doc.run_pending_callbacks
7879
doc.run_callbacks(:build) { doc }
7980
doc
8081
end

lib/mongoid/document.rb

Lines changed: 75 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -100,21 +100,8 @@ def identity
100100
# @param [ Hash ] attrs The attributes to set up the document with.
101101
#
102102
# @return [ Document ] A new document.
103-
def initialize(attrs = nil)
104-
@__parent = nil
105-
_building do
106-
@new_record = true
107-
@attributes ||= {}
108-
apply_pre_processed_defaults
109-
apply_default_scoping
110-
process_attributes(attrs) do
111-
yield(self) if block_given?
112-
end
113-
apply_post_processed_defaults
114-
# @todo: #2586: Need to have access to parent document in these
115-
# callbacks.
116-
run_callbacks(:initialize) unless _initialize_callbacks.empty?
117-
end
103+
def initialize(attrs = nil, &block)
104+
construct_document(attrs, execute_callbacks: true, &block)
118105
end
119106

120107
# Return the model name of the document.
@@ -228,6 +215,36 @@ def becomes(klass)
228215

229216
private
230217

218+
# Does the construction of a document.
219+
#
220+
# @param [ Hash ] attrs The attributes to set up the document with.
221+
# @param [ true | false ] execute_callbacks Flag specifies whether callbacks
222+
# should be run.
223+
#
224+
# @return [ Document ] A new document.
225+
#
226+
# @api private
227+
def construct_document(attrs = nil, execute_callbacks: true)
228+
@__parent = nil
229+
_building do
230+
@new_record = true
231+
@attributes ||= {}
232+
apply_pre_processed_defaults
233+
apply_default_scoping
234+
process_attributes(attrs) do
235+
yield(self) if block_given?
236+
end
237+
apply_post_processed_defaults
238+
239+
if execute_callbacks
240+
run_callbacks(:initialize) unless _initialize_callbacks.empty?
241+
else
242+
pending_callbacks << :initialize
243+
end
244+
end
245+
self
246+
end
247+
231248
# Returns the logger
232249
#
233250
# @return [ Logger ] The configured logger or a default Logger instance.
@@ -281,20 +298,58 @@ module ClassMethods
281298
# @param [ Hash ] attrs The hash of attributes to instantiate with.
282299
# @param [ Integer ] selected_fields The selected fields from the
283300
# criteria.
301+
# @param [ true | false ] execute_callbacks Flag specifies whether callbacks
302+
# should be run.
303+
#
304+
# @return [ Document ] A new document.
305+
def instantiate(attrs = nil, selected_fields = nil, &block)
306+
instantiate_document(attrs, selected_fields, execute_callbacks: true, &block)
307+
end
308+
309+
# Instantiate the document.
310+
#
311+
# @param [ Hash ] attrs The hash of attributes to instantiate with.
312+
# @param [ Integer ] selected_fields The selected fields from the
313+
# criteria.
314+
# @param [ true | false ] execute_callbacks Flag specifies whether callbacks
315+
# should be run.
284316
#
285317
# @return [ Document ] A new document.
286-
def instantiate(attrs = nil, selected_fields = nil)
318+
#
319+
# @api private
320+
def instantiate_document(attrs = nil, selected_fields = nil, execute_callbacks: true)
287321
attributes = attrs || {}
288322
doc = allocate
289323
doc.__selected_fields = selected_fields
290324
doc.instance_variable_set(:@attributes, attributes)
291-
doc.apply_defaults
292-
yield(doc) if block_given?
293-
doc.run_callbacks(:find) unless doc._find_callbacks.empty?
294-
doc.run_callbacks(:initialize) unless doc._initialize_callbacks.empty?
325+
326+
if execute_callbacks
327+
doc.apply_defaults
328+
yield(doc) if block_given?
329+
doc.run_callbacks(:find) unless doc._find_callbacks.empty?
330+
doc.run_callbacks(:initialize) unless doc._initialize_callbacks.empty?
331+
else
332+
yield(doc) if block_given?
333+
doc.pending_callbacks.push(:apply_defaults, :find, :initialize)
334+
end
335+
295336
doc
296337
end
297338

339+
# Allocates and constructs a document.
340+
#
341+
# @param [ Hash ] attrs The attributes to set up the document with.
342+
# @param [ true | false ] execute_callbacks Flag specifies whether callbacks
343+
# should be run.
344+
#
345+
# @return [ Document ] A new document.
346+
#
347+
# @api private
348+
def construct_document(attrs = nil, execute_callbacks: true)
349+
doc = allocate
350+
doc.send(:construct_document, attrs, execute_callbacks: execute_callbacks)
351+
end
352+
298353
# Returns all types to query for when using this class as the base.
299354
#
300355
# @example Get the types.

lib/mongoid/factory.rb

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,32 @@ module Factory
2323
#
2424
# @param [ Class ] klass The class to instantiate from if _type is not present.
2525
# @param [ Hash ] attributes The document attributes.
26+
# @param [ true | false ] execute_callbacks Flag specifies whether callbacks
27+
# should be run.
2628
#
2729
# @return [ Document ] The instantiated document.
2830
def build(klass, attributes = nil)
31+
execute_build(klass, attributes, execute_callbacks: true)
32+
end
33+
34+
# Execute the build.
35+
#
36+
# @param [ Class ] klass The class to instantiate from if _type is not present.
37+
# @param [ Hash ] attributes The document attributes.
38+
# @param [ true | false ] execute_callbacks Flag specifies whether callbacks
39+
# should be run.
40+
#
41+
# @return [ Document ] The instantiated document.
42+
#
43+
# @api private
44+
def execute_build(klass, attributes = nil, execute_callbacks: true)
2945
attributes ||= {}
3046
dvalue = attributes[klass.discriminator_key] || attributes[klass.discriminator_key.to_sym]
3147
type = klass.get_discriminator_mapping(dvalue)
3248
if type
33-
type.new(attributes)
49+
type.construct_document(attributes, execute_callbacks: execute_callbacks)
3450
else
35-
klass.new(attributes)
51+
klass.construct_document(attributes, execute_callbacks: execute_callbacks)
3652
end
3753
end
3854

@@ -63,12 +79,34 @@ def build(klass, attributes = nil)
6379
#
6480
# @return [ Document ] The instantiated document.
6581
def from_db(klass, attributes = nil, criteria = nil, selected_fields = nil)
82+
execute_from_db(klass, attributes, criteria, selected_fields, execute_callbacks: true)
83+
end
84+
85+
# Execute from_db.
86+
#
87+
# @param [ Class ] klass The class to instantiate from if _type is not present.
88+
# @param [ Hash ] attributes The document attributes.
89+
# @param [ Criteria ] criteria Optional criteria object.
90+
# @param [ Hash ] selected_fields Fields which were retrieved via
91+
# #only. If selected_fields are specified, fields not listed in it
92+
# will not be accessible in the returned document.
93+
# @param [ true | false ] execute_callbacks Whether this method should
94+
# invoke the callbacks. If true, the callbacks will be invoked normally.
95+
# If false, the callbacks will be stored in the +pending_callbacks+ list
96+
# and caller is responsible for invoking +run_pending_callbacks+ at a
97+
# later time. Use this option to defer callback execution until the
98+
# entire object graph containing embedded associations is constructed.
99+
#
100+
# @return [ Document ] The instantiated document.
101+
#
102+
# @api private
103+
def execute_from_db(klass, attributes = nil, criteria = nil, selected_fields = nil, execute_callbacks: true)
66104
if criteria
67105
selected_fields ||= criteria.options[:fields]
68106
end
69107
type = (attributes || {})[klass.discriminator_key]
70108
if type.blank?
71-
obj = klass.instantiate(attributes, selected_fields)
109+
obj = klass.instantiate_document(attributes, selected_fields, execute_callbacks: execute_callbacks)
72110
if criteria && criteria.association && criteria.parent_document
73111
obj.set_relation(criteria.association.inverse, criteria.parent_document)
74112
end
@@ -92,7 +130,7 @@ def from_db(klass, attributes = nil, criteria = nil, selected_fields = nil)
92130
raise Errors::UnknownModel.new(camelized, type)
93131
end
94132

95-
constantized.instantiate(attributes, selected_fields)
133+
constantized.instantiate_document(attributes, selected_fields, execute_callbacks: execute_callbacks)
96134
end
97135
end
98136
end

0 commit comments

Comments
 (0)