Merge branch 'master' of github.com:neo4jrb/neo4j-core

Author: cheerfulstoic

Gemfile

@@ -11,12 +11,7 @@ gem 'coveralls', require: false
 gem 'simplecov-html', require: false
 
 group 'development' do
-  gem 'pry-rescue', platform: :ruby
-  gem 'pry-stack_explorer', platform: :ruby
-
-  gem 'guard'
   gem 'guard-rspec', require: false
-  gem 'guard-rubocop'
 end
 
 group 'test' do

lib/neo4j-server/cypher_relationship.rb

@@ -105,7 +105,7 @@ def rel_type
       end
 
       def del
-        @session._query("#{match_start} DELETE n", neo_id: neo_id).raise_unless_response_code(200)
+        @session._query("#{match_start} DELETE n", neo_id: neo_id)
       end
       alias_method :delete, :del
       alias_method :destroy, :del

lib/neo4j-server/cypher_response.rb

@@ -130,7 +130,7 @@ def data?
       end
 
       def raise_unless_response_code(code)
-        fail "Response code #{response.code}, expected #{code} for #{response.request.path}, #{response.body}" unless response.status == code
+        fail "Response code #{response.status}, expected #{code} for #{response.headers['location']}, #{response.body}" unless response.status == code
       end
 
       def each_data_row

lib/neo4j-server/cypher_session.rb

@@ -105,7 +105,7 @@ def begin_tx
           # Handle nested transaction "placebo transaction"
           Neo4j::Transaction.current.push_nested!
         else
-          wrap_resource('transaction', CypherTransaction, :post, @connection)
+          wrap_resource(@connection)
         end
         Neo4j::Transaction.current
       end

lib/neo4j-server/cypher_transaction.rb

@@ -1,62 +1,78 @@
 module Neo4j
   module Server
+    # The CypherTransaction object lifecycle is as follows:
+    # * It is initialized with the transactional endpoint URL and the connection object to use for communication. It does not communicate with the server to create this.
+    # * The first query within the transaction sets the commit and execution addresses, :commit_url and :exec_url.
+    # * At any time, `failure` can be called to mark a transaction failed and trigger a rollback upon closure.
+    # * `close` is called to end the transaction. It calls `_commit_tx` or `_delete_tx`.
+    #
+    # If a transaction is created and then closed without performing any queries, an OpenStruct is returned that behaves like a successfully closed query.
     class CypherTransaction
       include Neo4j::Transaction::Instance
       include Neo4j::Core::CypherTranslator
       include Resource
 
-      attr_reader :commit_url, :exec_url
+      attr_reader :commit_url, :exec_url, :base_url, :connection
 
-      class CypherError < StandardError
-        attr_reader :code, :status
-        def initialize(code, status, message)
-          super(message)
-          @code = code
-          @status = status
-        end
-      end
-
-      def initialize(response, url, connection)
-        @connection = connection
-        @commit_url = response.body['commit']
-        @exec_url = response.headers['Location']
-        fail "NO ENDPOINT URL #{@connection} : HEAD: #{response.headers.inspect}" if !@exec_url || @exec_url.empty?
-        init_resource_data(response.body, url)
-        expect_response_code(response, 201)
+      def initialize(url, session_connection)
+        @base_url = url
+        @connection = session_connection
         register_instance
       end
 
+      ROW_REST = %w(row REST)
       def _query(cypher_query, params = nil)
-        statement = {statement: cypher_query, parameters: params, resultDataContents: %w(row REST)}
+        fail 'Transaction expired, unable to perform query' if expired?
+        statement = {statement: cypher_query, parameters: params, resultDataContents: ROW_REST}
         body = {statements: [statement]}
-        response = @connection.post(@exec_url, body)
+
+        response = exec_url && commit_url ? connection.post(exec_url, body) : register_urls(body)
         _create_cypher_response(response)
       end
 
+      def _delete_tx
+        _tx_query(:delete, exec_url, headers: resource_headers)
+      end
+
+      def _commit_tx
+        _tx_query(:post, commit_url, nil)
+      end
+
+      private
+
+      def _tx_query(action, endpoint, headers = {})
+        return empty_response if !commit_url || expired?
+        response = connection.send(action, endpoint, headers)
+        expect_response_code(response, 200)
+        response
+      end
+
+      def register_urls(body)
+        response = connection.post(base_url, body)
+        @commit_url = response.body['commit']
+        @exec_url = response.headers['Location']
+        fail "NO ENDPOINT URL #{connection} : HEAD: #{response.headers.inspect}" if !exec_url || exec_url.empty?
+        init_resource_data(response.body, base_url)
+        expect_response_code(response, 201)
+        response
+      end
+
       def _create_cypher_response(response)
         first_result = response.body['results'][0]
 
         cr = CypherResponse.new(response, true)
-        if !response.body['errors'].empty?
+        if response.body['errors'].empty?
+          cr.set_data(first_result['data'], first_result['columns'])
+        else
           first_error = response.body['errors'].first
+          expired if first_error['message'].match(/Unrecognized transaction id/)
           cr.set_error(first_error['message'], first_error['code'], first_error['code'])
-        else
-          cr.set_data(first_result['data'], first_result['columns'])
         end
         cr
       end
 
-      def _delete_tx
-        response = @connection.delete(@exec_url, headers: resource_headers)
-        expect_response_code(response, 200)
-        response
-      end
-
-      def _commit_tx
-        response = @connection.post(@commit_url)
-
-        expect_response_code(response, 200)
-        response
+      def empty_response
+        OpenStruct.new(status: 200, body: '')
       end
     end
   end

lib/neo4j-server/resource.rb

@@ -16,15 +16,9 @@ def init_resource_data(resource_data, resource_url)
         self
       end
 
-
-      def wrap_resource(key, resource_class, verb = :get, statement = {}, connection)
-        fail "Illegal verb #{verb}" if not [:get, :post].include?(verb)
-
-        url = resource_url(key)
-
-        response = connection.send(verb, url, statement.empty? ? nil : statement)
-
-        resource_class.new(response, url, connection) if response.status != 404
+      def wrap_resource(connection = Neo4j::Session.current)
+        url = resource_url('transaction')
+        CypherTransaction.new(url, connection)
       end
 
       def resource_url(key = nil)

lib/neo4j/transaction.rb

@@ -9,15 +9,26 @@ def register_instance
         Neo4j::Transaction.register(self)
       end
 
-      # Marks this transaction as failed, which means that it will unconditionally be rolled back when close() is called.
-      def failure
+      # Marks this transaction as failed, which means that it will unconditionally be rolled back when close() is called. Aliased for legacy purposes.
+      def mark_failed
         @failure = true
       end
+      alias_method :failure, :mark_failed
 
-      # If it has been marked as failed
-      def failure?
+      # If it has been marked as failed. Aliased for legacy purposes.
+      def failed?
         !!@failure
       end
+      alias_method :failure?, :failed?
+
+      def mark_expired
+        @expired = true
+      end
+      alias_method :expired, :mark_expired
+
+      def expired?
+        !!@expired
+      end
 
       # @private
       def push_nested!
@@ -51,16 +62,10 @@ def close
         return if @pushed_nested >= 0
         fail "Can't commit transaction, already committed" if @pushed_nested < -1
         Neo4j::Transaction.unregister(self)
-        if failure?
-          _delete_tx
-        else
-          _commit_tx
-        end
+        failed? ? _delete_tx : _commit_tx
       end
     end
 
-
-
     # @return [Neo4j::Transaction::Instance]
     def new(current = Session.current!)
       current.begin_tx
@@ -82,7 +87,7 @@ def run(run_in_tx = true)
           puts "Java Exception in a transaction, cause: #{e.cause}"
           e.cause.print_stack_trace
         end
-        tx.failure unless tx.nil?
+        tx.mark_failed unless tx.nil?
         raise
       ensure
         tx.close unless tx.nil?

spec/neo4j-server/e2e/cypher_transaction_spec.rb

@@ -10,62 +10,78 @@ module Server
         Neo4j::Transaction.current && Neo4j::Transaction.current.close
       end
 
-      it 'can open and commit a transaction' do
-        tx = session.begin_tx
-        tx.close
-      end
-
-      it 'can run a valid query' do
-        id = session.query.create('(n)').return('ID(n) AS id').first[:id]
+      context 'where no queries are made' do
+        it 'can open and close a transaction' do
+          tx = session.begin_tx
+          expect { tx.close }.not_to raise_error
+        end
 
-        tx = session.begin_tx
-        q = tx._query("MATCH (n) WHERE ID(n) = #{id} RETURN ID(n)")
-        expect(q.response.body['results']).to eq([{'columns' => ['ID(n)'], 'data' => [{'row' => [id], 'rest' => [id]}]}])
+        it 'returns an OpenStruct to mimic a completed transaction' do
+          tx = session.begin_tx
+          response = tx.close
+          expect(response.status).to eq(200)
+          expect(response).to be_a(OpenStruct)
+        end
       end
 
+      context 'where queries are made' do
+        it 'can open and close a transaction' do
+          tx = session.begin_tx
+          tx._query("CREATE (n:Student { name: 'John' } RETURN n")
+          response = tx.close
+          expect(response.status).to eq 200
+          expect(response).to be_a(Faraday::Response)
+        end
 
-      it 'sets the response error fields if not a valid query' do
-        tx = session.begin_tx
-        r = tx._query('START n=fs(0) RRETURN ID(n)')
-        expect(r.error?).to be true
-
-        expect(r.error_msg).to match(/Invalid input/)
-        expect(r.error_status).to match(/Syntax/)
-      end
+        it 'can run a valid query' do
+          id = session.query.create('(n)').return('ID(n) AS id').first[:id]
+          tx = session.begin_tx
+          q = tx._query("MATCH (n) WHERE ID(n) = #{id} RETURN ID(n)")
+          expect(q.response.body['results']).to eq([{'columns' => ['ID(n)'], 'data' => [{'row' => [id], 'rest' => [id]}]}])
+        end
 
-      it 'can commit' do
-        tx = session.begin_tx
-        response = tx.close
-        expect(response.status).to eq(200)
-      end
+        it 'sets the response error fields if not a valid query' do
+          tx = session.begin_tx
+          r = tx._query('START n=fs(0) RRETURN ID(n)')
+          expect(r.error?).to be true
 
-      it 'can rollback' do
-        node = Neo4j::Node.create(name: 'andreas')
-        Neo4j::Transaction.run do |tx|
-          node[:name] = 'foo'
-          expect(node[:name]).to eq('foo')
-          tx.failure
+          expect(r.error_msg).to match(/Invalid input/)
+          expect(r.error_status).to match(/Syntax/)
         end
 
-        expect(node['name']).to eq('andreas')
-      end
+        it 'can rollback' do
+          node = Neo4j::Node.create(name: 'andreas')
+          Neo4j::Transaction.run do |tx|
+            node[:name] = 'foo'
+            expect(node[:name]).to eq('foo')
+            tx.mark_failed
+          end
 
-      it 'can continue operations after transaction is rolled back' do
-        node = Neo4j::Node.create(name: 'andreas')
-        Neo4j::Transaction.run do |tx|
-          tx.failure
-          node[:name] = 'foo'
-          expect(node[:name]).to eq('foo')
+          expect(node['name']).to eq('andreas')
+        end
+
+        it 'can continue operations after transaction is rolled back' do
+          node = Neo4j::Node.create(name: 'andreas')
+          Neo4j::Transaction.run do |tx|
+            tx.mark_failed
+            node[:name] = 'foo'
+            expect(node[:name]).to eq('foo')
+          end
+          expect(node['name']).to eq('andreas')
         end
-        expect(node['name']).to eq('andreas')
-      end
 
-      it 'can use Transaction block style' do
-        node = Neo4j::Transaction.run do
-          Neo4j::Node.create(name: 'andreas')
+        it 'cannot continue operations if a transaction is expired' do
+          node = Neo4j::Node.create(name: 'andreas')
+          Neo4j::Transaction.run do |tx|
+            tx.expired
+            expect { node[:name] = 'foo' }.to raise_error 'Transaction expired, unable to perform query'
+          end
         end
 
-        expect(node['name']).to eq('andreas')
+        it 'can use Transaction block style' do
+          node = Neo4j::Transaction.run { Neo4j::Node.create(name: 'andreas') }
+          expect(node['name']).to eq('andreas')
+        end
       end
 
       describe Neo4j::Label do
@@ -140,8 +156,8 @@ module Server
             loaded = a.rel(dir: :outgoing, type: :knows)
             expect(loaded).to eq(rel)
             expect(loaded['colour']).to eq('blue')
-            ensure
-              tx.close
+          ensure
+            tx.close
           end
         end
       end
@@ -158,7 +174,6 @@ module Server
 
       describe '#del' do
         it 'deletes a node' do
-          skip 'see https://github.com/neo4j/neo4j/issues/2943'
           begin
             tx = session.begin_tx
             node = Neo4j::Node.create(name: 'andreas')
@@ -172,7 +187,6 @@ module Server
         end
       end
 
-
       describe '#[]=' do
         it 'can update/read a property' do
           node = Neo4j::Node.create(name: 'foo')