Skip to content
This repository
Browse code

Fix for "Save callbacks fire once for every outging has_one node" clo…

…ses #172

The write performance will probably be increase since the size of the call stack has been decrease a lot since we are avoiding saving nodes twice.
This might also be a solution for issue #166
  • Loading branch information...
commit ec6401924e14cb4f06c7495ab2bcfbed3cdf2407 1 parent 0724e56
Andreas Ronge authored
10 lib/neo4j/rails/persistence.rb
@@ -119,6 +119,11 @@ def destroy_all
119 119 end
120 120 end
121 121
  122 + # Returns if the entity is currently being updated or created
  123 + def create_or_updating?
  124 + !!@_create_or_updating
  125 + end
  126 +
122 127 protected
123 128
124 129 def update
@@ -128,6 +133,8 @@ def update
128 133 end
129 134
130 135 def create_or_update
  136 + # since the same model can be created or updated twice from a relationship we have to have this guard
  137 + @_create_or_updating = true
131 138 result = persisted? ? update : create
132 139 unless result != false
133 140 Neo4j::Rails::Transaction.fail if Neo4j::Rails::Transaction.running?
@@ -135,8 +142,11 @@ def create_or_update
135 142 else
136 143 true
137 144 end
  145 + ensure
  146 + @_create_or_updating = nil
138 147 end
139 148
  149 +
140 150 def set_deleted_properties
141 151 @_deleted = true
142 152 end
2  lib/neo4j/rails/relationship_persistence.rb
@@ -104,7 +104,7 @@ def end_node=(node)
104 104 end
105 105
106 106 def _persist_node(start_or_end_node)
107   - (start_or_end_node.new_record? || start_or_end_node.relationships_changed?) ? start_or_end_node.save : true
  107 + ((start_or_end_node.new_record? || start_or_end_node.relationships_changed?) && !start_or_end_node.create_or_updating?) ? start_or_end_node.save : true
108 108 end
109 109
110 110 end
1  spec/integration/versioning/versioning_spec.rb
@@ -128,7 +128,6 @@ class MaxVersion < Neo4j::Rails::Model
128 128 end
129 129
130 130 it "restores an older version with relationships" do
131   - pending "Does not work with new active model 3.2"
132 131 ferarri = SportsCar.create!(:name => 'Ferarri')
133 132 ferarri.version(1).incoming(:sports_cars).should be_empty
134 133 porsche = SportsCar.create!(:name => 'Porsche')
27 spec/regressions/issue_172_spec.rb
... ... @@ -0,0 +1,27 @@
  1 +module Regressions
  2 + require 'spec_helper'
  3 +
  4 + describe "Issue 172, Save callbacks fire once for every outgoing has_one node" do
  5 + it "should only call save callback once" do
  6 + a = Neo4j::Rails::Model.new
  7 + b = Neo4j::Rails::Model.new
  8 +
  9 + klass = create_model do
  10 + has_one(:target1)
  11 + has_one(:target2)
  12 + after_save :foobar
  13 +
  14 + def foobar
  15 + end
  16 + end
  17 +
  18 + c = klass.new
  19 + c.should_receive(:foobar).once
  20 + c.target1 = a
  21 + c.target2 = b
  22 + c.save
  23 + end
  24 +
  25 + end
  26 +
  27 +end
3  spec/unit/has_n_spec.rb
@@ -112,6 +112,9 @@ def wrapper
112 112
113 113 node.should_receive(:save).and_return(true)
114 114 other.should_receive(:save).and_return(true)
  115 + node.should_receive(:create_or_updating?).and_return(false)
  116 + other.should_receive(:create_or_updating?).and_return(false)
  117 +
115 118 node.write_changed_relationships
116 119
117 120 new_relationship.start_node.should == node
15 spec/unit/has_one_spec.rb
@@ -147,12 +147,27 @@ def wrapper
147 147
148 148 node.should_receive(:save).and_return(true)
149 149 other.should_receive(:save).and_return(true)
  150 + node.should_receive(:create_or_updating?).and_return(false)
  151 + other.should_receive(:create_or_updating?).and_return(false)
  152 +
150 153 node.write_changed_relationships
151 154
152 155 new_relationship.start_node.should == node
153 156 new_relationship.end_node.should == other
154 157 end
155 158 end
  159 +
  160 + it "should not store the relationship twice" do
  161 + node.stub(:new_record?).and_return(true)
  162 + other.stub(:new_record?).and_return(true)
  163 +
  164 + node.should_not_receive(:save)
  165 + other.should_not_receive(:save)
  166 + node.should_receive(:create_or_updating?).and_return(true)
  167 + other.should_receive(:create_or_updating?).and_return(true)
  168 +
  169 + node.write_changed_relationships
  170 + end
156 171 end
157 172 end
158 173 end
7 spec/unit/relationships_spec.rb
@@ -120,10 +120,15 @@ def self.to_s
120 120
121 121 describe "write_changed_relationships" do
122 122 before do
123   - other_node.should_receive(:save).and_return(true)
  123 + node.should_receive(:create_or_updating?).and_return(false)
124 124 node.should_receive(:save).and_return(true)
  125 +
  126 + other_node.should_receive(:create_or_updating?).and_return(true)
  127 + other_node.should_not_receive(:save)
  128 +
125 129 node.write_changed_relationships
126 130
  131 +
127 132 node.stub(:persisted?).and_return(true)
128 133 node.stub(:new_record?).and_return(false)
129 134 end

0 comments on commit ec64019

Please sign in to comment.
Something went wrong with that request. Please try again.