RestMixin no longer requires Lucene index on classname [#60 state:res…

…olved]

lib/neo4j/extensions/rest.rb

@@ -9,6 +9,9 @@
 require 'neo4j/extensions/rest/stubs'
 require 'neo4j/extensions/rest/server'
 
+# Provides Neo4j::NodeMixin::ClassMethods#all
+require 'neo4j/extensions/reindexer'
+
 
 module Neo4j
   # Make the ReferenceNode available as a REST resource

lib/neo4j/extensions/rest/rest_mixin.rb

@@ -43,15 +43,6 @@ def _uri_rel
       clazz = self.class.root_class.to_s #.gsub(/::/, '-') TODO urlencoding
       "/nodes/#{clazz}/#{neo_node_id}"
     end
-
-    def initialize(*args)
-      super
-      # Explicitly index the classname of a node (required for <code>GET /nodes/MyClass</code>
-      # Lucene search to work).
-      self.class.indexer.on_property_changed(self, 'classname')   # TODO reuse the event_handler instead !
-      # This caused the replication_spec.rb to fail
-     # Neo4j.event_handler.property_changed(self, 'classname', '', self.class.to_s)
-    end
 
     # Called by the REST API if this node is accessed directly by ID. Any query parameters
     # in the request are passed in a hash. For example if <code>GET /nodes/MyClass/1?foo=bar</code>
@@ -69,8 +60,6 @@ def delete(options={})
 
 
     def self.included(c)
-      c.property :classname
-      c.index :classname # index classname so that we can search on it
       c.extend ClassMethods
       uri_rel = c._uri_rel
       # just for debugging and logging purpose so we know which classes uses this mixin, TODO - probablly not needed
@@ -94,59 +83,104 @@ def find(query=nil, &block)
 
         query = symbolize_keys(query)
 
-        if query[:limit]
-          limit = query[:limit].to_s.split(/,/).map{|i| i.to_i}
-          limit.unshift(0) if limit.size == 1
+        if query[:search]
+          # Use Lucene
+          results = super(query[:search])
+          results = apply_lucene_sort(query[:sort], results).to_a rescue super(query[:search]).to_a
+
+        else
+          # Use traverser
+          results = apply_ruby_sort(query[:sort], apply_traverser_conditions(query))
         end
 
-        # Build search query
-        search = query[:search]
-        if search.nil?
-          search = {:classname => self.name}
-          query.each_pair do |key, value|
-            search[key.to_sym] = value unless [:sort, :limit].include? key.to_sym
-          end
+        apply_limits(query[:limit], results)
+      end
+
+      # :nodoc:
+      def symbolize_keys(hash)
+        # Borrowed from ActiveSupport
+        hash.inject({}) do |options, (key, value)|
+          options[(key.to_sym rescue key) || key] = value
+          options
         end
+      end
+
+      protected
 
-        # Add sorting to the mix
-        if query[:sort]
-          last_field = nil
-          results = super(search)
-          query[:sort].split(/,/).each do |field|
-            if %w(asc desc).include? field
-              results = results.sort_by(field == 'asc' ? Lucene::Asc[last_field] : Lucene::Desc[last_field])
-              last_field = nil
-            else
-              results = results.sort_by(Lucene::Asc[last_field]) unless last_field.nil?
-              last_field = field
+      # Searches for nodes matching conditions by using a traverser.
+      def apply_traverser_conditions(query)
+        query = query.reject{|key, value| [:sort, :limit, :classname].include? key }
+
+        index_node = Neo4j::IndexNode.instance
+        raise 'Index node is nil. Make sure you have called Neo4j.load_reindexer' if index_node.nil?
+        traverser = index_node.traverse.outgoing(root_class)
+
+        traverser.filter do |position|
+          node = position.current_node
+          position.depth == 1 and
+            query.inject(true) do |meets_condition, (key, value)|
+              meets_condition && (node.send(key) == value)
+            end
+        end
+      end
+
+      # Sorts a list of results according to a string of comma-separated fieldnames (optionally
+      # with 'asc' or 'desc' thrown in). For use in cases where we don't go via Lucene.
+      def apply_ruby_sort(sort_string, results)
+        if sort_string
+          sort_fields = sort_string.to_s.split(/,/)
+          results.to_a.sort do |x,y|
+            catch(:item_order) do
+              sort_fields.each_index do |index|
+                field = sort_fields[index]
+                unless %w(asc desc).include?(field)
+                  item_order = if sort_fields[index + 1] == 'desc'
+                    (y.send(field) || '') <=> (x.send(field) || '')
+                  else
+                    (x.send(field) || '') <=> (y.send(field) || '')
+                  end
+                  throw :item_order, item_order unless item_order == 0
+                end
+              end
+              0
             end
-          end
-          results = results.sort_by(Lucene::Asc[last_field]) unless last_field.nil?
-          begin
-            results = results.to_a
-          rescue NativeException => e
-            results = super(search).to_a
           end
         else
-          results = super(search).to_a
+          results.to_a
         end
+      end
+
+      # Applies Lucene sort instructions to a Neo4j::SearchResult object.
+      def apply_lucene_sort(sort_string, results)
+        return results if sort_string.nil?
+        last_field = nil
+
+        sort_string.to_s.split(/,/).each do |field|
+          if %w(asc desc).include? field
+            results = results.sort_by(field == 'asc' ? Lucene::Asc[last_field] : Lucene::Desc[last_field])
+            last_field = nil
+          else
+            results = results.sort_by(Lucene::Asc[last_field]) unless last_field.nil?
+            last_field = field
+          end
+        end
+        results.sort_by(Lucene::Asc[last_field]) unless last_field.nil?
+        results
+      end
+
+      # Return only the requested subset of results for pagination
+      # (TODO: can this be done more efficiently within Lucene?)
+      def apply_limits(limit_string, results)
+        if limit_string
+          limit = limit_string.to_s.split(/,/).map{|i| i.to_i}
+          limit.unshift(0) if limit.size == 1
 
-        # Return only the requested subset of results (TODO: can this be done more efficiently within Lucene?)
-        if limit
           (limit[0]...(limit[0]+limit[1])).map{|n| results[n] }
         else
           results
         end
       end
 
-      # :nodoc:
-      def symbolize_keys(hash)
-        # Borrowed from ActiveSupport
-        hash.inject({}) do |options, (key, value)|
-          options[(key.to_sym rescue key) || key] = value
-          options
-        end
-      end
     end
   end
 

test/rest/rest_spec.rb

@@ -58,6 +58,7 @@ class MyNode
       include Neo4j::RestMixin
     end
     Neo4j.start
+    Neo4j.load_reindexer
     Neo4j::Transaction.new
   end
 
@@ -118,13 +119,14 @@ class FooRest
   end
 
   it "should traverse a relationship on GET nodes/RestPerson/<id>/traverse?relationship=friends&depth=1" do
-    adam = RestPerson.new
+    # the reference node has id = 0; the index node has id = 1
+    adam = RestPerson.new # neo_node_id = 2
     adam.name = 'adam'
 
-    bertil = RestPerson.new
+    bertil = RestPerson.new # neo_node_id = 3
     bertil.name = 'bertil'
 
-    carl = RestPerson.new
+    carl = RestPerson.new # neo_node_id = 4
 
     adam.friends << bertil << carl
 
@@ -135,8 +137,8 @@ class FooRest
     last_response.status.should == 200
     body = JSON.parse(last_response.body)
     body['uri_list'].should_not be_nil
-    body['uri_list'][0].should == 'http://0.0.0.0:4567/nodes/RestPerson/2'
-    body['uri_list'][1].should == 'http://0.0.0.0:4567/nodes/RestPerson/3'
+    body['uri_list'][0].should == 'http://0.0.0.0:4567/nodes/RestPerson/3' # bertil
+    body['uri_list'][1].should == 'http://0.0.0.0:4567/nodes/RestPerson/4' # carl
     body['uri_list'].size.should == 2
   end
 
@@ -153,7 +155,13 @@ class FooRest
 
     # then
     last_response.status.should == 201
-    last_response.location.should == "/relationships/1" # starts counting from 0
+    # rel ID 0 = reference node to index node
+    # rel ID 1 = index node to adam
+    # rel ID 2 = index node to bertil
+    # rel ID 3 = index node to unnamed
+    # rel ID 4 = bertil to unnamed
+    # rel ID 5 = adam to bertil (the one we just created)
+    last_response.location.should == "/relationships/5"
     adam.friends.should include(bertil)
   end
 
@@ -205,8 +213,9 @@ class FooRest
   end
 
   it "should be possible to load a relationship on GET /relationship/<id>" do
-    adam = RestPerson.new
-    bertil = RestPerson.new
+    # the reference node has id = 0; the index node has id = 1
+    adam = RestPerson.new # neo_node_id = 2
+    bertil = RestPerson.new # neo_node_id = 3
     rel = adam.friends.new(bertil)
     rel[:foo] = 'bar'
 
@@ -217,8 +226,8 @@ class FooRest
     last_response.status.should == 200
     body = JSON.parse(last_response.body)
     body['properties']['foo'].should == 'bar'
-    body['end_node']['uri'].should == 'http://0.0.0.0:4567/nodes/RestPerson/2'
-    body['start_node']['uri'].should == 'http://0.0.0.0:4567/nodes/RestPerson/1' 
+    body['end_node']['uri'].should == 'http://0.0.0.0:4567/nodes/RestPerson/3' # bertil
+    body['start_node']['uri'].should == 'http://0.0.0.0:4567/nodes/RestPerson/2' # adam
   end
 
 
@@ -230,7 +239,7 @@ class FooRest
 
     # then
     last_response.status.should == 201
-    last_response.location.should == "http://0.0.0.0:4567/nodes/RestPerson/1"
+    last_response.location.should == "http://0.0.0.0:4567/nodes/RestPerson/2" # 0 is ref node, 1 is index node
   end
 
   it "should persist a new RestPerson created by POST /nodes/RestPerson" do
@@ -280,15 +289,15 @@ class FooRest
 
 
   it "should contain hyperlinks to its relationships on found nodes" do
-    # given
-    n1 = MyNode.new
-    n2 = MyNode.new
-    n3 = MyNode.new
-    n4 = MyNode.new
+    # given         # rel ID 0: reference node -> index node
+    n1 = MyNode.new # rel ID 1: index -> n1
+    n2 = MyNode.new # rel ID 2: index -> n2
+    n3 = MyNode.new # rel ID 3: index -> n3
+    n4 = MyNode.new # rel ID 4: index -> n4
 
-    n1.relationships.outgoing(:type1) << n2
-    n1.relationships.outgoing(:type2) << n3
-    n1.relationships.outgoing(:type2) << n4
+    n1.relationships.outgoing(:type1) << n2 # rel ID 5
+    n1.relationships.outgoing(:type2) << n3 # rel ID 6
+    n1.relationships.outgoing(:type2) << n4 # rel ID 7
 
     # when
     get "/nodes/MyNode/#{n1.neo_node_id}"
@@ -299,9 +308,9 @@ class FooRest
     data['relationships'].should_not be_nil
     data['relationships']['type1'].should_not be_nil
     data['relationships']['type2'].should_not be_nil
-    data['relationships']['type2'].should include('http://0.0.0.0:4567/relationships/1')
-    data['relationships']['type2'].should include('http://0.0.0.0:4567/relationships/2')
-    data['relationships']['type1'].should include('http://0.0.0.0:4567/relationships/0')
+    data['relationships']['type2'].should include('http://0.0.0.0:4567/relationships/6')
+    data['relationships']['type2'].should include('http://0.0.0.0:4567/relationships/7')
+    data['relationships']['type1'].should include('http://0.0.0.0:4567/relationships/5')
   end