Fix Ruby client seqid handling
diff --git a/lib/rb/spec/client_spec.rb b/lib/rb/spec/client_spec.rb
index 8650350..c963b47 100644
--- a/lib/rb/spec/client_spec.rb
+++ b/lib/rb/spec/client_spec.rb
@@ -24,6 +24,11 @@
include Thrift::Client
end
+ class EmptyArgs
+ def write(_prot)
+ end
+ end
+
before(:each) do
@prot = double("MockProtocol")
@client = ClientSpec.new(@prot)
@@ -52,15 +57,34 @@
end
it "should increment the sequence id when sending messages" do
- pending "it seems sequence ids are completely ignored right now"
- @prot.expect(:write_message_begin).with('testMessage', Thrift::MessageTypes::CALL, 0).ordered
- @prot.expect(:write_message_begin).with('testMessage2', Thrift::MessageTypes::CALL, 1).ordered
- @prot.expect(:write_message_begin).with('testMessage3', Thrift::MessageTypes::CALL, 2).ordered
- @prot.stub!(:write_message_end)
- @prot.stub!(:trans).and_return double("trans").as_null_object
- @client.send_message('testMessage', double("args class").as_null_object)
- @client.send_message('testMessage2', double("args class").as_null_object)
- @client.send_message('testMessage3', double("args class").as_null_object)
+ expect(@prot).to receive(:write_message_begin).with('testMessage', Thrift::MessageTypes::CALL, 0).ordered
+ expect(@prot).to receive(:write_message_begin).with('testMessage2', Thrift::MessageTypes::CALL, 1).ordered
+ expect(@prot).to receive(:write_message_begin).with('testMessage3', Thrift::MessageTypes::CALL, 2).ordered
+ allow(@prot).to receive(:write_message_end)
+ allow(@prot).to receive(:trans).and_return(double("trans", :flush => nil))
+
+ args_class = double("ArgsClass", :new => EmptyArgs.new)
+ @client.send_message('testMessage', args_class)
+ @client.send_message('testMessage2', args_class)
+ @client.send_message('testMessage3', args_class)
+ end
+
+ it "should keep pending reply sequence ids in FIFO order" do
+ expect(@prot).to receive(:write_message_begin).with('first', Thrift::MessageTypes::CALL, 0).ordered
+ expect(@prot).to receive(:write_message_begin).with('second', Thrift::MessageTypes::CALL, 1).ordered
+ allow(@prot).to receive(:write_message_end)
+ allow(@prot).to receive(:trans).and_return(double("trans", :flush => nil))
+
+ args_class = double("ArgsClass", :new => EmptyArgs.new)
+ @client.send_message('first', args_class)
+ @client.send_message('second', args_class)
+
+ expect {
+ @client.validate_message_begin('first', Thrift::MessageTypes::REPLY, 0, 'first')
+ }.not_to raise_error
+ expect {
+ @client.validate_message_begin('second', Thrift::MessageTypes::REPLY, 1, 'second')
+ }.not_to raise_error
end
it "should receive a test message" do
@@ -72,16 +96,59 @@
@client.receive_message(double("MockClass", :new => mock_klass))
end
- it "should handle received exceptions" do
- expect(@prot).to receive(:read_message_begin).and_return [nil, Thrift::MessageTypes::EXCEPTION, 0]
+ it "should raise BAD_SEQUENCE_ID for mismatched replies" do
+ @client.instance_variable_set(:@pending_seqids, [0])
+
+ expect {
+ @client.validate_message_begin('testMessage', Thrift::MessageTypes::REPLY, 1, 'testMessage')
+ }.to raise_error(Thrift::ApplicationException) { |error|
+ expect(error.type).to eq(Thrift::ApplicationException::BAD_SEQUENCE_ID)
+ }
+ end
+
+ it "should raise WRONG_METHOD_NAME for unexpected replies" do
+ @client.instance_variable_set(:@pending_seqids, [0])
+
+ expect {
+ @client.validate_message_begin('otherMessage', Thrift::MessageTypes::REPLY, 0, 'testMessage')
+ }.to raise_error(Thrift::ApplicationException) { |error|
+ expect(error.type).to eq(Thrift::ApplicationException::WRONG_METHOD_NAME)
+ }
+ end
+
+ it "should raise INVALID_MESSAGE_TYPE for non-reply messages" do
+ @client.instance_variable_set(:@pending_seqids, [0])
+
+ expect {
+ @client.validate_message_begin('testMessage', Thrift::MessageTypes::CALL, 0, 'testMessage')
+ }.to raise_error(Thrift::ApplicationException) { |error|
+ expect(error.type).to eq(Thrift::ApplicationException::INVALID_MESSAGE_TYPE)
+ }
+ end
+
+ it "should raise received application exceptions" do
expect(@prot).to receive(:read_message_end)
- expect(Thrift::ApplicationException).to receive(:new) do
- StandardError.new.tap do |mock_exc|
- expect(mock_exc).to receive(:read).with(@prot)
- end
- end
- fname, mtype, sqeid = @client.receive_message_begin()
- expect { @client.handle_exception(mtype) }.to raise_error(StandardError)
+ server_exception = Thrift::ApplicationException.new(Thrift::ApplicationException::UNKNOWN, "boom")
+ expect(server_exception).to receive(:read).with(@prot)
+ expect(Thrift::ApplicationException).to receive(:new).and_return(server_exception)
+ @client.instance_variable_set(:@pending_seqids, [0])
+
+ expect {
+ @client.validate_message_begin('testMessage', Thrift::MessageTypes::EXCEPTION, 0, 'testMessage')
+ }.to raise_error(Thrift::ApplicationException, "boom")
+ expect(@client.instance_variable_get(:@pending_seqids)).to be_empty
+ end
+
+ it "should roll sequence ids across the signed int32 boundary" do
+ expect(@prot).to receive(:write_message_begin).with('testMessage', Thrift::MessageTypes::CALL, Thrift::Client::MAX_SEQUENCE_ID).ordered
+ expect(@prot).to receive(:write_message_begin).with('testMessage2', Thrift::MessageTypes::CALL, Thrift::Client::MIN_SEQUENCE_ID).ordered
+ allow(@prot).to receive(:write_message_end)
+ allow(@prot).to receive(:trans).and_return(double("trans", :flush => nil))
+
+ @client.instance_variable_set(:@seqid, Thrift::Client::MAX_SEQUENCE_ID)
+ args_class = double("ArgsClass", :new => EmptyArgs.new)
+ @client.send_message('testMessage', args_class)
+ @client.send_message('testMessage2', args_class)
end
it "should close the transport if an error occurs while sending a message" do