Merge pull request #78 from vivekprahlad/master

Fix for rule node concurrency issue [#200]
neo4jrb · Dec 2, 2011 · 7620586 · 7620586
2 parents a6b1f06 + 2cf1ef5
commit 7620586
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 37 deletions.
diff --git a/lib/neo4j/rule/rule_node.rb b/lib/neo4j/rule/rule_node.rb
@@ -10,6 +10,7 @@ class RuleNode
       include ToJava
       attr_reader :rules
       attr_reader :model_class
+      @@rule_nodes = {}
 
       def initialize(clazz)
         @model_class = eval("#{clazz}")
@@ -28,6 +29,20 @@ def node_exist?
         !ref_node.rel?(@classname)
       end
 
+      def rule_node
+        ref_node._java_node.synchronized do
+          @@rule_nodes[key] ||= find_node || create_node
+        end
+      end
+
+      def rule_node?(node)
+        @@rule_nodes[key] == node
+      end
+
+      def key
+        "#{ref_node}#{@ref_node_key}".to_sym
+      end
+
       def ref_node
         if @model_class.respond_to? :ref_node_for_class
           @model_class.ref_node_for_class
@@ -61,11 +76,6 @@ def find_node
         ref_node.rel?(@classname.to_s) && ref_node._rel(:outgoing, @classname.to_s)._end_node
       end
 
-      def rule_node
-        clear_rule_node if ref_node_changed?
-        Thread.current[@rule_node_key] ||= find_node || create_node
-      end
-
       def ref_node_changed?
         if ref_node != Thread.current[@ref_node_key]
           Thread.current[@ref_node_key] = ref_node
@@ -75,16 +85,8 @@ def ref_node_changed?
         end
       end
 
-      def rule_node?(node)
-        cached_rule_node == node
-      end
-
-      def cached_rule_node
-        Thread.current[@rule_node_key]
-      end
-
       def clear_rule_node
-        Thread.current[@rule_node_key] = nil
+        @@rule_nodes[key] = nil
       end
 
       def rule_names

diff --git a/spec/rule/rule_spec.rb b/spec/rule/rule_spec.rb
@@ -30,6 +30,7 @@ class NewsStory
   rule(:young_readers) { !readers.find { |user| !user.young? } }
 end
 
+
 describe "Neo4j::Node#rule", :type => :transactional do
 
   it "generate instance method: <rule_name>? for each rule" do
@@ -74,6 +75,25 @@ class NewsStory
     Reader.young.should include(b)
   end
 
+    #Run this test alone to reproduce the issue
+
+  it "rule node created from concurrent threads" do
+    Neo4j.threadlocal_ref_node = nil
+    Reader.all.each(&:del)
+    finish_tx
+    readers = []
+    threads = 2.times.collect do
+      Thread.new do
+        Neo4j.threadlocal_ref_node = nil
+        new_tx
+        readers << Reader.new(:age => 2)
+        finish_tx
+      end
+     end
+    threads.each(&:join)
+    Reader.all.to_a.should =~ readers
+  end
+
   it "rule only instances of the given class (no side effects)" do
     Reader.new :age => 25
     Reader.new :age => 4
@@ -253,28 +273,5 @@ class NewsStory
         FastReader.all.should include(subject)
       end
     end
-
-    context "allow rule nodes to be attached to a defined reference node" do
-      after(:each) { Neo4j.threadlocal_ref_node = nil}
-
-      it "should allow the reference node for a class to be defined via a block" do
-        new_tx
-          other_reference_node = Neo4j::Node.new
-        finish_tx
-
-        class ReferenceNodeExample
-          include Neo4j::NodeMixin
-          rule :all
-          ref_node { Neo4j.default_ref_node }
-        end
-
-        new_tx
-          example = ReferenceNodeExample.new(:name => "Name")
-        finish_tx
-        Neo4j.threadlocal_ref_node = other_reference_node
-
-        ReferenceNodeExample.all.size.should == 1
-      end
-    end
   end
 end