Lazy association creation

[subvertallchris]

This is far from ideal but it will hopefully get the ball rolling in regard to working with associations and unpersisted nodes.

It makes a few key changes that are demonistrated in https://github.com/neo4jrb/neo4j/blob/lazy_association_creation/spec/e2e/unpersisted_association_spec.rb.

The basics are simple: when adding nodes to a has_many or has_one rel using << or =, it checks whether the nodes are persisted. If they are not, it will stash the nodes to associate in the unpersisted node's association_cache. When save is called in the node, it checks for the presence of deferred associations and, if it finds any, it processes them after save completes.

It makes a few significant changes. In this case:

student = Student.create
lesson = Lesson.new
student.lessons << lesson

save will be called on the lesson immediately and the association will be processed. I know we debated this before but this is consistent with ActiveRecord.

In this case:

student = Student.new
less = Lesson.new
student.lessons << lesson
student.save

...that final save will trigger a cascading save: first student, then lesson, then the association is created. It will all occur within a transaction, so a failure of any piece of the operation will roll back the others. If lesson was an existing node and student was unpersisted, it would wait for student.save to create the relationship and it would avoid the extra save on lesson.

This could probably use a full more tests, so I'm going to come back to it tomorrow; in particular, I want to make sure that it works correctly when you give it an array or multiple nodes in separate method calls.

My biggest complaint here is that the whole process is rather inefficient. Each operation happens within a separate trip to the DB, so there's no real performance benefit when compared to manually managing transactions and the order of operations.

But there are benefits. The best thing this does is let you worry less about the order of your operations and the transaction handling is convenient. Performing multiple operations within a transaction is a bit faster than independent operations, so this will be safer and faster than what many people might be doing.

Aside from improving coverage a little, the next steps would be:

• Investigate whether the process should reuse any ActiveRecord method names and signatures
• Add methods that rely on this process to improve compatibility with third-party gems. An autosave association option and a QueryProxy build method immediately come to mind.

[subvertallchris]

Interesting, this is failing on Neo4j 2.1.7?

[subvertallchris]

ping @cheerfulstoic and @dpisarewski
Dieter, this changes your work from a few months ago that let a user automatically save unpersisted nodes when associating. It changes to the default, essentially, and removes the option to disable it.

[olance]

Is Rubocop really OK about this?!

[subvertallchris]

I guess so. It's really sensitive about the length of things, so I tend to try pushing out before down.

[olance]

yup, it's just that I've had an error on something similar once, but actually it was on a parallel assignment.

[olance]

@subvertallchris have you deleted your last comment? ^^

I meant to chime in earlier but got quite busy lately... so apparently you've debated about this already, but could you give the rational behind automatically saving lesson if it's added to another persisted object's collection?
Does ActiveRecord really do that too?

Anyway, I'd say it's already a great improvement and I'd be happy to be able to use it :)

[subvertallchris]

Haha, you caught me! I didn't want to be too pushy... publicly. ;-) I poked Brian directly cause his internet connection is iffy at the moment.

Believe it or not, ActiveRecord does really immediately save lesson. I've always been in the "raise-an-error-and-make-them-save" camp but our previous debates showed me as the minority. General consensus was that if you try to do that, you expect to be dealing with a persisted or persistable node, so calling save first is reasonable.

[olance]

hmm ok, I thought we could do nothing but add the lesson to the collection and save it only when cascading saves from the student.save, otherwise I don't really see the point of having to call it!
Calling save automatically on lesson but not saving the relationship and requiring to call save on student to do so, makes little sense to me ^^

[subvertallchris]

It really depends on the state of all of the nodes. If student is unpersisted, nothing happens until you call save. If student is persisted, adding to its lessons will call save on the lesson... but all is not lost. You can always work in reverse.

lesson = Lesson.new
student = Student.first
lesson.students << student
# save has not been called anywhere

lesson.save
# now a save will trigger on student and the rel will be created

So there's that. As far as I can see, the basics mirror ActiveRecord, but it's possible there's a configuration option available that changes this or maybe I misinterpreted something along the way.

[subvertallchris]

Oh, and in this case:

student = Student.first
lesson = Lesson.new
student.lessons << lesson

That will immediately save the lesson and create the relationship, no save required.

[olance]

oh, yeah, sorry I had mixed the two examples when reading back your post!
So I guess that's cool :)

[cheerfulstoic]

Tsk tsk... ;)

[subvertallchris]

This was done before I split things out into modules, it can be removed
now. ;-)

On Friday, July 17, 2015, Brian Underwood [email protected] wrote:

In .rubocop_todo.yml
#871 (comment):

@@ -16,7 +16,7 @@ Metrics/AbcSize:

Offense count: 2

Configuration parameters: CountComments.

Metrics/ClassLength:

• Max: 208
• Max: 214

Tsk tsk... ;)

—
Reply to this email directly or view it on GitHub
https://github.com/neo4jrb/neo4j/pull/871/files#r34912584.

[cheerfulstoic]

Seems pretty straightforward! Thanks for this!

[subvertallchris]

Very cool. Thanks for checking it out, guys. Even though it's somewhat simple, it feels like a big change to established behavior, so I appreciate having the extra sets of eyes on it.