From 0726323110870e22f44a87be999e044791ffa0fa Mon Sep 17 00:00:00 2001 From: Max De Marzi Date: Sat, 15 Jun 2013 16:53:09 -0500 Subject: [PATCH] catching errors in batch and propagating up --- lib/neography/connection.rb | 53 ++++++++++++++++++++-------- lib/neography/rest/batch.rb | 16 +++------ spec/integration/rest_batch_spec.rb | 22 +++++------- spec/integration/rest_header_spec.rb | 2 +- spec/spec_helper.rb | 7 ++++ spec/unit/connection_spec.rb | 2 +- spec/unit/rest/batch_spec.rb | 42 +++++++++++----------- 7 files changed, 81 insertions(+), 63 deletions(-) diff --git a/lib/neography/connection.rb b/lib/neography/connection.rb index dcc71b8..7085f90 100644 --- a/lib/neography/connection.rb +++ b/lib/neography/connection.rb @@ -30,8 +30,9 @@ def configuration end def merge_options(options) - merged_options = options.merge!(@authentication)#.merge!(@parser) + merged_options = options.merge!(@authentication) merged_options[:headers].merge!(@user_agent) if merged_options[:headers] + merged_options[:headers].merge!('X-Stream' => true) if merged_options[:headers] merged_options end @@ -121,19 +122,36 @@ def evaluate_chunk_response(response, result) end def evaluate_response(response) - code = response.code - body = response.body - return_result(code, body) + if response.http_header.request_uri.request_uri == "/db/data/batch" + code, body, parsed = handle_batch(response) + else + code = response.code + body = response.body + parsed = false + end + return_result(code, body, parsed) + end + + def handle_batch(response) + code = 200 + body = @parser.json(response.body) + body.each do |result| + if result["status"] >= 400 + code = result["status"] + break + end + end + return code, body, true end - def return_result(code, body) + def return_result(code, body, parsed) case code when 200 - @logger.debug "OK" if @log_enabled - @parser.json(body) #response.parsed_response + @logger.debug "OK, created #{body}" if @log_enabled + parsed ? body : @parser.json(body) when 201 @logger.debug "OK, created #{body}" if @log_enabled - r = @parser.json(body) #response.parsed_response + r = parsed ? body : @parser.json(body) r.extend(WasCreated) r when 204 @@ -147,19 +165,24 @@ def return_result(code, body) def handle_4xx_500_response(code, body) if body.nil? or body == "" - parsed_body = {} - message = "No error message returned from server." - stacktrace = "" + parsed_body = {"message" => "No error message returned from server.", + "stacktrace" => "No stacktrace returned from server." } elsif body.is_a? Hash parsed_body = body - message = parsed_body["message"] - stacktrace = parsed_body["stacktrace"] + elsif body.is_a? Array + body.each do |result| + if result["status"] >= 400 + parsed_body = result["body"] || result + break + end + end else parsed_body = @parser.json(body) - message = parsed_body["message"] - stacktrace = parsed_body["stacktrace"] end + message = parsed_body["message"] + stacktrace = parsed_body["stacktrace"] + @logger.error "#{code} error: #{body}" if @log_enabled case code diff --git a/lib/neography/rest/batch.rb b/lib/neography/rest/batch.rb index ca32233..d29a1c6 100644 --- a/lib/neography/rest/batch.rb +++ b/lib/neography/rest/batch.rb @@ -11,29 +11,21 @@ def initialize(connection) end def execute(*args) - batch({'Accept' => 'application/json;stream=true'}, *args) - end - - def not_streaming(*args) - batch({}, *args) + batch(*args) end private - def batch(accept_header, *args) + def batch(*args) batch = [] Array(args).each_with_index do |c, i| batch << {:id => i }.merge(get_batch(c)) end options = { :body => batch.to_json, - :headers => json_content_type.merge(accept_header) + :headers => json_content_type } - if accept_header.empty? - @connection.post(batch_path, options) - else - @connection.post_chunked(batch_path, options) - end + @connection.post(batch_path, options) end def get_batch(args) diff --git a/spec/integration/rest_batch_spec.rb b/spec/integration/rest_batch_spec.rb index 30bb854..2c71c5d 100644 --- a/spec/integration/rest_batch_spec.rb +++ b/spec/integration/rest_batch_spec.rb @@ -445,38 +445,34 @@ # See https://github.com/neo4j/community/issues/697 expect { - batch_result = @neo.batch [:create_unique_node, "person", "ssn", "000-00-0001", {:first_name=>"Jane", :last_name=>"Doe", :ssn=>"000-00-0001", :_type=>"Person", :created_at=>1335269478}], - [:add_node_to_index, "person_ssn", "ssn", "000-00-0001", "{0}"], - [:create_node, {:street1=>"94437 Kemmer Crossing", :street2=>"Apt. 333", :city=>"Abshireton", :state=>"AA", :zip=>"65820", :_type=>"Address", :created_at=>1335269478}], - [:create_relationship, "has", "{0}", "{2}", {}] - }.to raise_error(Neography::NeographyError) + batch_result = @neo.batch [:create_unique_node, "person", "ssn", "000-00-0001", {:first_name=>"Jane", :last_name=>"Doe", :ssn=>"000-00-0001", :_type=>"Person", :created_at=>1335269478}], + [:add_node_to_index, "person_ssn", "ssn", "000-00-0001", "{0}"], + [:create_node, {:street1=>"94437 Kemmer Crossing", :street2=>"Apt. 333", :city=>"Abshireton", :state=>"AA", :zip=>"65820", :_type=>"Address", :created_at=>1335269478}], + [:create_relationship, "has", "{0}", "{2}", {}] + }.to raise_error(Neography::NeographyError) end end describe "broken queries" do it "should return errors when bad syntax is passed in batch" do - batch_commands = [] - # batch_commands << [ :create_unique_node, "person", "ssn", "000-00-0002", {:foo => "bar"} ] - # this doesn't raise error batch_commands << [ :execute_query, "start person_n=node:person(ssn = '000-00-0002') - set bar1 = {foo}", + set bar1 = {foo}", { :other => "what" } ] + expect { + batch_result = @neo.batch *batch_commands + }.to raise_exception Neography::SyntaxException - # this does raise error expect { @neo.execute_query("start person_n=node:person(ssn = '000-00-0001') set bar = {foo}", { :other => "what" }) }.to raise_exception Neography::SyntaxException - expect { - batch_result = @neo.batch *batch_commands - }.to raise_exception Neography::SyntaxException end end diff --git a/spec/integration/rest_header_spec.rb b/spec/integration/rest_header_spec.rb index 3005cf6..c88188a 100644 --- a/spec/integration/rest_header_spec.rb +++ b/spec/integration/rest_header_spec.rb @@ -8,7 +8,7 @@ it "should add a content type if there's existing headers" do subject.merge_options({:headers => {'Content-Type' => 'foo/bar'}})[:headers].should == - {'Content-Type' => "foo/bar", "User-Agent" => "Neography/#{Neography::VERSION}"} + {'Content-Type' => "foo/bar", "User-Agent" => "Neography/#{Neography::VERSION}" , "X-Stream"=>true} end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7dd6fe8..8c65cf0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -26,7 +26,14 @@ def json_content_type end def error_response(attributes) + request_uri = double() + request_uri.stub(:request_uri).and_return("") + + http_header = double() + http_header.stub(:request_uri).and_return(request_uri) + stub( + http_header: http_header, code: attributes[:code], body: { message: attributes[:message], diff --git a/spec/unit/connection_spec.rb b/spec/unit/connection_spec.rb index b78bcab..bd58363 100644 --- a/spec/unit/connection_spec.rb +++ b/spec/unit/connection_spec.rb @@ -123,7 +123,7 @@ module Neography it "adds the User-Agent to the headers" do connection.client.should_receive(:get).with( "http://localhost:7474/db/data/foo/bar", - nil, { "User-Agent" => "Neography/#{Neography::VERSION}" } + nil, { "User-Agent" => "Neography/#{Neography::VERSION}", "X-Stream"=>true} ) { stub.as_null_object } connection.get("/foo/bar", :headers => {}) diff --git a/spec/unit/rest/batch_spec.rb b/spec/unit/rest/batch_spec.rb index cfd69d5..d937465 100644 --- a/spec/unit/rest/batch_spec.rb +++ b/spec/unit/rest/batch_spec.rb @@ -13,7 +13,7 @@ class Rest { "id" => 1, "method" => "GET", "to" => "/node/bar" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:get_node, "foo"], [:get_node, "bar"] end @@ -23,7 +23,7 @@ class Rest { "id" => 1, "method" => "POST", "to" => "/node", "body" => { "baz" => "qux" } } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:create_node, { "foo" => "bar" }], [:create_node, { "baz" => "qux" }] end @@ -33,7 +33,7 @@ class Rest { "id" => 1, "method" => "DELETE", "to" => "/node/bar" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:delete_node, "foo"], [:delete_node, "bar"] end @@ -43,7 +43,7 @@ class Rest { "id" => 1, "method" => "POST", "to" => "/index/node/quux?unique", "body" => { "key" => "corge", "value" => "grault", "properties" => "garply" } } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:create_unique_node, "foo", "bar", "baz", "qux" ], [:create_unique_node, "quux", "corge", "grault", "garply"] end @@ -54,7 +54,7 @@ class Rest { "id" => 1, "method" => "POST", "to" => "/index/node/quux", "body" => { "uri" => "{0}", "key" => "corge", "value" => "grault" } } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:add_node_to_index, "foo", "bar", "baz", "qux" ], [:add_node_to_index, "quux", "corge", "grault", "{0}"] end @@ -65,7 +65,7 @@ class Rest { "id" => 1, "method" => "GET", "to" => "/index/node/qux/quux/corge" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:get_node_index, "foo", "bar", "baz" ], [:get_node_index, "qux", "quux", "corge" ] end @@ -77,7 +77,7 @@ class Rest { "id" => 2, "method" => "DELETE", "to" => "/index/node/index3/key3/value3/id3" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:remove_node_from_index, "index1", "id1", ], [:remove_node_from_index, "index2", "key2", "id2" ], [:remove_node_from_index, "index3", "key3", "value3", "id3" ] @@ -89,7 +89,7 @@ class Rest { "id" => 1, "method" => "PUT", "to" => "/node/index2/properties/key2", "body" => "value2" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:set_node_property, "index1", { "key1" => "value1" } ], [:set_node_property, "index2", { "key2" => "value2" } ] end @@ -100,7 +100,7 @@ class Rest { "id" => 1, "method" => "PUT", "to" => "/node/index2/properties", "body" => { "key2" => "value2" } } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:reset_node_properties, "index1", { "key1" => "value1" } ], [:reset_node_properties, "index2", { "key2" => "value2" } ] end @@ -111,7 +111,7 @@ class Rest { "id" => 1, "method" => "GET", "to" => "/node/id2/relationships/all" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:get_node_relationships, "id1", "direction1" ], [:get_node_relationships, "id2" ] end @@ -122,7 +122,7 @@ class Rest { "id" => 1, "method" => "GET", "to" => "/relationship/bar" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:get_relationship, "foo"], [:get_relationship, "bar"] end @@ -132,7 +132,7 @@ class Rest { "id" => 1, "method" => "POST", "to" => "{0}/relationships", "body" => { "to" => "{1}", "type" => "type2", "data" => "data2" } } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:create_relationship, "type1", "from1", "to1", "data1" ], [:create_relationship, "type2", "{0}", "{1}", "data2" ] end @@ -143,7 +143,7 @@ class Rest { "id" => 1, "method" => "DELETE", "to" => "/relationship/bar" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:delete_relationship, "foo"], [:delete_relationship, "bar"] end @@ -153,7 +153,7 @@ class Rest { "id" => 1, "method" => "POST", "to" => "/index/relationship/index2?unique", "body" => { "key" => "key2", "value" => "value2", "type" => "type2", "start" => "{0}", "end" => "{1}" } } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:create_unique_relationship, "index1", "key1", "value1", "type1", "node1", "node2" ], [:create_unique_relationship, "index2", "key2", "value2", "type2", "{0}", "{1}" ] end @@ -164,7 +164,7 @@ class Rest { "id" => 1, "method" => "POST", "to" => "/index/relationship/index2", "body" => { "uri" => "{0}", "key" => "key2", "value" => "value2" } } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:add_relationship_to_index, "index1", "key1", "value1", "rel1" ], [:add_relationship_to_index, "index2", "key2", "value2", "{0}"] end @@ -175,7 +175,7 @@ class Rest { "id" => 1, "method" => "GET", "to" => "/index/relationship/qux/quux/corge" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:get_relationship_index, "foo", "bar", "baz" ], [:get_relationship_index, "qux", "quux", "corge" ] end @@ -187,7 +187,7 @@ class Rest { "id" => 2, "method" => "DELETE", "to" => "/index/relationship/index3/key3/value3/id3" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:remove_relationship_from_index, "index1", "id1", ], [:remove_relationship_from_index, "index2", "key2", "id2" ], [:remove_relationship_from_index, "index3", "key3", "value3", "id3" ] @@ -199,7 +199,7 @@ class Rest { "id" => 1, "method" => "PUT", "to" => "/relationship/index2/properties/key2", "body" => "value2" } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:set_relationship_property, "index1", { "key1" => "value1" } ], [:set_relationship_property, "index2", { "key2" => "value2" } ] end @@ -210,7 +210,7 @@ class Rest { "id" => 1, "method" => "PUT", "to" => "{0}/properties", "body" => { "key2" => "value2" } } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:reset_relationship_properties, "index1", { "key1" => "value1" } ], [:reset_relationship_properties, "{0}", { "key2" => "value2" } ] end @@ -221,7 +221,7 @@ class Rest { "id" => 1, "method" => "POST", "to" => "/gremlin", "body" => { "script" => "script2", "params" => "params2" } } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:execute_script, "script1", "params1"], [:execute_script, "script2", "params2"] end @@ -232,7 +232,7 @@ class Rest { "id" => 1, "method" => "POST", "to" => "/cypher", "body" => { "query" => "query2" } } ] - connection.should_receive(:post_chunked).with("/batch", json_match(:body, expected_body)) + connection.should_receive(:post).with("/batch", json_match(:body, expected_body)) subject.execute [:execute_query, "query1", "params1"], [:execute_query, "query2" ] end