-
Notifications
You must be signed in to change notification settings - Fork 59
handle cyclic associations #26
base: master
Are you sure you want to change the base?
Conversation
…ct reload when resolving possible
model_class = constantize(type) | ||
translate_ids type, id, attributes | ||
begin | ||
new_id, instance = model_class.load_replicant(type, id, attributes) | ||
if not local_id.nil? | ||
new_id, instance = model_class.load_replicant(type, id, attributes, local_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing the local_id
here is going to break backward compatibility unfortunately. Existing programs that implement custom load_replicant
method will raise an ArgumentError.
Thanks for this. I've run into the problem a few times now. I need to do a bit closer review of the diff and approach but I'm definitely interested in landing something that fixes this. /cc @jbarnette |
Any update on this? why not just giving a default |
Anything new on this? I just cut a new fork where I've merged this into it, but would prefer to track the gem from the source. |
@@ -16,6 +16,7 @@ class Loader < Emitter | |||
|
|||
def initialize | |||
@keymap = Hash.new { |hash,k| hash[k] = {} } | |||
@wait = Hash.new { |hash,k| hash[k] = [] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this does go forward, this should be a Hash
so non Integer
ids don't error.
Conflicts: test/active_record_test.rb
…ences objects, check for overridden load_replicant for backward compat
Trying to revive this, merged the rtomayko's current master branch and made two changes:
It would be nice if you could test dump & restore in (non-critical ,-) environments of your real world applications. |
cyclic associations lead to an infinite loop on dump, for instance
I suggest two changes which I have implemented as a PoC: