Skip to content

Commit

Permalink
catching errors in batch and propagating up
Browse files Browse the repository at this point in the history
  • Loading branch information
maxdemarzi committed Jun 15, 2013
1 parent e8f9b2b commit 0726323
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 63 deletions.
53 changes: 38 additions & 15 deletions lib/neography/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 4 additions & 12 deletions lib/neography/rest/batch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 9 additions & 13 deletions spec/integration/rest_batch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/rest_header_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/connection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {})
Expand Down
42 changes: 21 additions & 21 deletions spec/unit/rest/batch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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" ]
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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" ]
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 0726323

Please sign in to comment.