Made unpersisted relationships available in callbacks [#211 state:res…

…olved]

Notice, if you add relationship to a model in a callback then the callback will be called twice.

lib/neo4j/rails/rel_persistence.rb

@@ -151,16 +151,19 @@ def update
       def create()
         begin
           # prevent calling create twice
-          @start_node.rm_outgoing_rel(type, self)
-          @end_node.rm_incoming_rel(type, self)
+          @start_node.add_unpersisted_outgoing_rel(type, self)
+          @end_node.add_unpersisted_incoming_rel(type, self)
 
-          _persist_start_node
-          _persist_end_node
+          return unless _persist_start_node && _persist_end_node
 
           @_java_rel = Neo4j::Relationship.new(type, start_node, end_node)
           Neo4j::IdentityMap.add(@_java_rel, self)
           init_on_create
           clear_changes
+
+          @start_node.rm_unpersisted_outgoing_rel(type, self)
+          @end_node.rm_unpersisted_incoming_rel(type, self)
+
         end unless @end_node.nil? or @start_node.nil?
         true
       end
@@ -170,21 +173,14 @@ def _load(id)
       end
 
       def _persist_start_node
-        if !@start_node.persisted? || @start_node.relationships_changed?
-          unless @start_node.save
-            raise "Can't save start_node #{@start_node} id #{@start_node.id}"
-          end
-        end
+        (!@start_node.persisted? || @start_node.relationships_changed?) ? @start_node.save : true
       end
 
       def _persist_end_node
-        if !@end_node.persisted? || @end_node.relationships_changed?
-          unless @end_node.save
-            raise "Can't save end_node #{@end_node} id #{@end_node.id}"
-          end
-        end
+        (!@end_node.persisted? || @end_node.relationships_changed?) ? @end_node.save : true
       end
 
+
       def init_on_create(*)
         #self["_classname"] = self.class.to_s
         write_default_attributes

lib/neo4j/rails/relationships/relationships.rb

@@ -30,6 +30,9 @@ def _create_or_get_storage_for_decl_rels(decl_rels) #:nodoc:
         @_relationships[decl_rels.rel_type.to_sym] ||= Storage.new(self, decl_rels.rel_type, decl_rels)
       end
 
+      def _storage_for(rel_type) #:nodoc:
+        @_relationships[rel_type.to_sym]
+      end
 
       # If the node is persisted and it does not have any unsaved relationship it returns a Neo4j::NodeTraverser.
       # Otherwise it will return a NodesDSL which behaves like the Neo4j::NodeTraverser except that it does not
@@ -90,6 +93,37 @@ def rm_outgoing_rel(rel_type, rel) #:nodoc:
         _create_or_get_storage(rel_type).rm_outgoing_rel(rel)
       end
 
+      def rm_outgoing_rel(rel_type, rel) #:nodoc:
+        _create_or_get_storage(rel_type).rm_outgoing_rel(rel)
+      end
+
+      def add_unpersisted_incoming_rel(rel_type, rel) #:nodoc
+        storage = _storage_for(rel_type)
+        # move the relationship since we are now about to store the relationship
+        storage.add_unpersisted_incoming_rel(rel)
+        storage.rm_incoming_rel(rel)
+      end
+
+      def add_unpersisted_outgoing_rel(rel_type, rel) #:nodoc
+        storage = _storage_for(rel_type)
+        # move the relationship since we are now about to store the relationship
+        storage.add_unpersisted_outgoing_rel(rel)
+        storage.rm_outgoing_rel(rel)
+      end
+
+      def rm_unpersisted_outgoing_rel(rel_type, rel)  #:nodoc
+        if (storage = _storage_for(rel_type))
+          storage.rm_unpersisted_outgoing_rel(rel)
+        end
+      end
+
+      def rm_unpersisted_incoming_rel(rel_type, rel)  #:nodoc
+        if (storage = _storage_for(rel_type))
+          storage.rm_unpersisted_incoming_rel(rel)
+        end
+      end
+
+
     end
   end
 end

lib/neo4j/rails/relationships/storage.rb

@@ -20,17 +20,21 @@ def initialize(node, rel_type, dsl)
         end
 
         def to_s #:nodoc:
-          "Storage #{object_id} node: #{@node.id} rel_type: #{@rel_type} outgoing #{@outgoing_rels.size} incoming #{@incoming_rels.size}"
+          "Storage #{object_id} node: #{@node.id} rel_type: #{@rel_type} outgoing #{@outgoing_rels.size}/#{@unpersisted_outgoing_rels && @unpersisted_outgoing_rels.size} incoming #{@incoming_rels.size}/#{@unpersisted_incoming_rels && @unpersisted_incoming_rels.size}"
         end
 
         def clear_unpersisted
           @outgoing_rels.clear
           @incoming_rels.clear
+          @unpersisted_outgoing_rels = nil
+          @unpersisted_incoming_rels = nil
         end
 
         def remove_from_identity_map
           @outgoing_rels.each {|r| Neo4j::IdentityMap.remove(r._java_rel)}
           @incoming_rels.each {|r| Neo4j::IdentityMap.remove(r._java_rel)}
+          @unpersisted_outgoing_rels = nil
+          @unpersisted_incoming_rels = nil
         end
 
         def size(dir)
@@ -57,9 +61,9 @@ def create(attrs)
         def relationships(dir)
           case dir
             when :outgoing
-              @outgoing_rels
+              @unpersisted_outgoing_rels || @outgoing_rels
             when :incoming
-              @incoming_rels
+              @unpersisted_incoming_rels || @incoming_rels
             when :both
               @incoming_rels + @outgoing_rels
           end
@@ -160,6 +164,18 @@ def add_outgoing_rel(rel)
           @outgoing_rels << rel
         end
 
+        # Makes the given relationship available in callbacks
+        def add_unpersisted_incoming_rel(rel)
+          @unpersisted_incoming_rels ||= []
+          @unpersisted_incoming_rels << rel
+        end
+
+        # Makes the given relationship available in callbacks
+        def add_unpersisted_outgoing_rel(rel)
+          @unpersisted_outgoing_rels ||= []
+          @unpersisted_outgoing_rels << rel
+        end
+
         def rm_incoming_rel(rel)
           @incoming_rels.delete(rel)
         end
@@ -168,6 +184,16 @@ def rm_outgoing_rel(rel)
           @outgoing_rels.delete(rel)
         end
 
+        def rm_unpersisted_incoming_rel(rel)
+          @unpersisted_incoming_rels.delete(rel)
+          @unpersisted_incoming_rels = nil if @unpersisted_incoming_rels.empty?
+        end
+
+        def rm_unpersisted_outgoing_rel(rel)
+          @unpersisted_outgoing_rels.delete(rel)
+          @unpersisted_outgoing_rels = nil if @unpersisted_outgoing_rels.empty?
+        end
+
         def persisted?
           @outgoing_rels.empty? && @incoming_rels.empty?
         end
@@ -179,14 +205,12 @@ def persist
           [@outgoing_rels, @incoming_rels, @persisted_related_nodes, @persisted_node_to_relationships, @persisted_relationships].each{|c| c.clear}
 
           out_rels.each do |rel|
-            rel.end_node.rm_incoming_rel(@rel_type.to_sym, rel) if rel.end_node
             success = rel.persisted? || rel.save
             # don't think this can happen - just in case, TODO
             raise "Can't save outgoing #{rel}, validation errors ? #{rel.errors.inspect}" unless success
           end
 
           in_rels.each do |rel|
-            rel.start_node.rm_outgoing_rel(@rel_type.to_sym, rel) if rel.start_node
             success = rel.persisted? || rel.save
             # don't think this can happen - just in case, TODO
             raise "Can't save incoming #{rel}, validation errors ? #{rel.errors.inspect}" unless success

spec/rails/cascade_save_spec.rb

@@ -1,44 +1,8 @@
 require File.join(File.dirname(__FILE__), '..', 'spec_helper')
 
 
-describe "Neo4j::Rails Cascade delete with callbacks" do
-#	subject { FindableModel.create!(:name => "Test 1", :age => 4241) }
+describe "Neo4j::Rails cascade update of models inside callbacks" do
 
-  class CascadePerson < Neo4j::Rails::Model
-    property :name
-    has_n :friends
-  end
-
-  class CascadeGroup < Neo4j::Rails::Model
-    has_n :people
-  end
-
-  class CascadeCompany < Neo4j::Rails::Model
-    has_n(:groups)
-    has_n :people
-    has_one :leader
-
-    before_save :cascade_add
-
-    def cascade_add
-      p1 = CascadePerson.create(:name => 'p1')
-
-      self.people << p1
-
-      p2 = CascadePerson.create(:name => 'p2')
-      p1.friends << p2
-    end
-  end
-
-  it "should allow to create new nodes in a rails callback" do
-    c = CascadeCompany.create!
-
-    c.people.size.should == 1
-    c.people.first.friends.size.should == 1
-#        first.should == c
-  end
-
-  #TODO: Consider moving the spec above under the example model below
   describe "example model" do
     let!(:person) do
       create_model
@@ -59,8 +23,8 @@ def people
     end
 
 
-    let(:brin)   { person.new }
-    let(:larry)  { person.new }
+    let(:brin) { person.create }
+    let(:larry) { person.new }
     let(:google) { company.new(google_data) }
 
     subject { google }
@@ -80,20 +44,20 @@ def people
 
         context "with leader passed in" do
           let(:google_data) { {:leader => brin} }
-          its(:valid?)      { should be_true }
-          its(:people)      { should include brin }
+          its(:valid?) { should be_true }
+          its(:people) { should include brin }
         end
 
         context "with validation on a leader :)" do
-          before            { company.validates(:leader, :presence=>true) }
+          before { company.validates(:leader, :presence=>true) }
           let(:google_data) { {:leader => nil} }
-          its(:valid?)      { should be_false }
-          its(:people)      { should be_empty }
+          its(:valid?) { should be_false }
+          its(:people) { should be_empty }
 
           context "and validating before saving" do
-            before            { google.valid? }
+            before { google.valid? }
             let(:google_data) { {:leader => brin} }
-            its(:persisted?)  { should be_true }
+            its(:persisted?) { should be_true }
           end
 
           context "when saving root with persisted nested" do
@@ -103,8 +67,8 @@ def people
               google.leader = brin
               google.save!
             end
-            its(:persisted?)  { should be_true }
-            its(:leader)      { should == brin }
+            its(:persisted?) { should be_true }
+            its(:leader) { should == brin }
           end
 
           context "when saving twice" do
@@ -120,8 +84,5 @@ def people
       end
 
     end
-
   end
-
 end
-