From 81c8473bba9d8ff5f148d4876d5021cb2f04a066 Mon Sep 17 00:00:00 2001 From: = Date: Thu, 1 Dec 2011 22:55:00 -0700 Subject: [PATCH 01/50] add eventedserver and socket server --- Gemfile.lock | 4 +++ lib/protobuf.rb | 5 ++- lib/protobuf/rpc/server.rb | 25 +------------- lib/protobuf/rpc/servers/evented_server.rb | 29 +++++++++++++++++ lib/protobuf/rpc/servers/socket_server.rb | 38 ++++++++++++++++++++++ protobuf.gemspec | 1 + spec/helper/server.rb | 2 +- spec/unit/server_spec.rb | 5 ++- 8 files changed, 80 insertions(+), 29 deletions(-) create mode 100644 lib/protobuf/rpc/servers/evented_server.rb create mode 100644 lib/protobuf/rpc/servers/socket_server.rb diff --git a/Gemfile.lock b/Gemfile.lock index 3269618e..dadb6216 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -3,12 +3,14 @@ PATH specs: protobuf (1.0.0) eventmachine (~> 0.12.10) + socketpool GEM remote: http://rubygems.org/ specs: diff-lcs (1.1.2) eventmachine (0.12.10) + eventmachine (0.12.10-java) rake (0.8.7) rspec (2.7.0) rspec-core (~> 2.7.0) @@ -18,8 +20,10 @@ GEM rspec-expectations (2.7.0) diff-lcs (~> 1.1.2) rspec-mocks (2.7.0) + socketpool (0.1.4) PLATFORMS + java ruby DEPENDENCIES diff --git a/lib/protobuf.rb b/lib/protobuf.rb index dc8795b6..dd935107 100644 --- a/lib/protobuf.rb +++ b/lib/protobuf.rb @@ -1,3 +1,5 @@ +require 'socket' +require 'socketpool' require 'eventmachine' require 'protobuf/ext/eventmachine' @@ -5,5 +7,6 @@ module Protobuf end require 'protobuf/rpc/client' -require 'protobuf/rpc/server' require 'protobuf/rpc/service' +require 'protobuf/rpc/servers/socket_server' +require 'protobuf/rpc/servers/evented_server' diff --git a/lib/protobuf/rpc/server.rb b/lib/protobuf/rpc/server.rb index 53402712..57e31e94 100644 --- a/lib/protobuf/rpc/server.rb +++ b/lib/protobuf/rpc/server.rb @@ -1,5 +1,3 @@ -require 'eventmachine' -require 'socket' require 'protobuf/common/logger' require 'protobuf/rpc/rpc.pb' require 'protobuf/rpc/buffer' @@ -8,26 +6,7 @@ module Protobuf module Rpc - class Server < EventMachine::Connection - include Protobuf::Logger::LogMethods - - # Initialize a new read buffer for storing client request info - def post_init - log_debug '[server] Post init, new read buffer created' - - @stats = Protobuf::Rpc::Stat.new(:SERVER, true) - @stats.client = Socket.unpack_sockaddr_in(get_peername) - - @buffer = Protobuf::Rpc::Buffer.new :read - @did_respond = false - end - - # Receive a chunk of data, potentially flushed to handle_client - def receive_data data - log_debug '[server] receive_data: %s' % data - @buffer << data - handle_client if @buffer.flushed? - end + module Server # Invoke the service method dictated by the proto wrapper request object def handle_client @@ -57,8 +36,6 @@ def handle_client send_response end - private - # Assuming all things check out, we can call the service method def invoke_rpc_method # Get a new instance of the service diff --git a/lib/protobuf/rpc/servers/evented_server.rb b/lib/protobuf/rpc/servers/evented_server.rb new file mode 100644 index 00000000..dcc5509a --- /dev/null +++ b/lib/protobuf/rpc/servers/evented_server.rb @@ -0,0 +1,29 @@ +require 'protobuf/rpc/server' + +module Protobuf + module Rpc + class EventedServer < EventMachine::Connection + include Protobuf::Rpc::Server + include Protobuf::Logger::LogMethods + + # Initialize a new read buffer for storing client request info + def post_init + log_debug '[server] Post init, new read buffer created' + + @stats = Protobuf::Rpc::Stat.new(:SERVER, true) + @stats.client = Socket.unpack_sockaddr_in(get_peername) + + @buffer = Protobuf::Rpc::Buffer.new :read + @did_respond = false + end + + # Receive a chunk of data, potentially flushed to handle_client + def receive_data data + log_debug '[server] receive_data: %s' % data + @buffer << data + handle_client if @buffer.flushed? + end + + end + end +end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb new file mode 100644 index 00000000..d1ff20b0 --- /dev/null +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -0,0 +1,38 @@ +require 'protobuf/rpc/server' + +module Protobuf + module Rpc + class SocketServer + include Protobuf::Rpc::Server + include Protobuf::Logger::LogMethods + + class << self + def run(host = "127.0.0.1", port = 9399) + server = TCPServer.new(host, port) + server.listen(100) + + loop do + Thread.new(server.accept) do |sock| + service_worker = self.new(sock) + sock.close + end + end + end + end + + def initialize(sock) + @did_response = false + @socket = sock + @request = Protobuf::Socketrpc::Request.new + @response = Protobuf::Socketrpc::Response.new + @buffer = Protobuf::Rpc::Buffer.new(:read) + @stats = Protobuf::Rpc::Stat.new(:SERVER, true) + + @stats.client = Socket.unpack_sockaddr_in(sock.getpeername) + @buffer << @socket.read + handle_client + end + + end + end +end diff --git a/protobuf.gemspec b/protobuf.gemspec index 94e9ca62..f692df07 100644 --- a/protobuf.gemspec +++ b/protobuf.gemspec @@ -19,6 +19,7 @@ Gem::Specification.new do |s| s.require_paths = ["lib"] s.add_dependency 'eventmachine', '~> 0.12.10' + s.add_dependency 'socketpool' s.add_development_dependency 'rake', '~> 0.8.7' s.add_development_dependency 'rspec', '~> 2.7.0' diff --git a/spec/helper/server.rb b/spec/helper/server.rb index a12925a3..a0d7a8e4 100644 --- a/spec/helper/server.rb +++ b/spec/helper/server.rb @@ -3,7 +3,7 @@ module StubProtobufServerFactory def self.build(delay) - new_server = Class.new(Protobuf::Rpc::Server) do + new_server = Class.new(Protobuf::Rpc::EventedServer) do class << self def sleep_interval @sleep_interval diff --git a/spec/unit/server_spec.rb b/spec/unit/server_spec.rb index 447bb574..fab2c005 100644 --- a/spec/unit/server_spec.rb +++ b/spec/unit/server_spec.rb @@ -1,11 +1,10 @@ require 'spec_helper' -require 'protobuf/rpc/server' require 'spec/proto/test_service_impl' describe Protobuf::Rpc::Server do context 'when sending response objects' do it 'should be able to send a hash object as a response' do - server = Protobuf::Rpc::Server.new + server = Protobuf::Rpc::EventedServer.new # Setup the right mocks server.instance_variable_set(:@klass, Spec::Proto::TestService) @@ -24,4 +23,4 @@ server.send(:parse_response_from_service, hash_response) end end -end \ No newline at end of file +end From c0a53f21e24d6b4d35f4d73805a597951bdcdf70 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 2 Dec 2011 00:28:52 -0700 Subject: [PATCH 02/50] add running and command line options for rpc_server to user socket server when requested --- bin/rpc_server | 51 ++++++++----------- lib/protobuf.rb | 18 ++++--- lib/protobuf/common/logger.rb | 4 +- lib/protobuf/rpc/servers/evented_runner.rb | 36 +++++++++++++ lib/protobuf/rpc/servers/socket_runner.rb | 22 ++++++++ lib/protobuf/rpc/servers/socket_server.rb | 16 ++++-- ...{server_spec.rb => evented_server_spec.rb} | 0 spec/unit/socket_server_spec.rb | 26 ++++++++++ 8 files changed, 130 insertions(+), 43 deletions(-) create mode 100644 lib/protobuf/rpc/servers/evented_runner.rb create mode 100644 lib/protobuf/rpc/servers/socket_runner.rb rename spec/unit/{server_spec.rb => evented_server_spec.rb} (100%) create mode 100644 spec/unit/socket_server_spec.rb diff --git a/bin/rpc_server b/bin/rpc_server index 4d43adf8..6b0c6a5e 100755 --- a/bin/rpc_server +++ b/bin/rpc_server @@ -3,16 +3,9 @@ require 'optparse' require 'ostruct' require 'logger' -require 'protobuf' -require 'protobuf/rpc/server' - -[:INT, :QUIT, :TERM].each do |sig| - trap(sig) do - EventMachine.stop_event_loop if EventMachine.reactor_running? - Protobuf::Logger.info 'Shutdown complete' - $stdout.puts 'Shutdown complete' - end -end +require 'protobuf/version' +require 'protobuf/rpc/servers/evented_runner' +require 'protobuf/rpc/servers/socket_runner' # Default options server = OpenStruct.new({ @@ -22,6 +15,7 @@ server = OpenStruct.new({ :port => 9595, :log => File.expand_path('./protobuf.log'), :level => ::Logger::INFO, + :runner => Protobuf::Rpc::EventedRunner, :debug => false }) @@ -47,6 +41,11 @@ parser = OptionParser.new do |opts| opts.on("-v N", "--level=N", Integer, "Log level to use, 0-5 (see http://www.ruby-doc.org/stdlib/libdoc/logger/rdoc/)") do |v| server.level = v.to_i end + + opts.on("-s", "--socket", "Socket Server Mode (No EventMachine)") do |v| + Protobuf::ServerType = "SocketServer" + server.runner = Protobuf::Rpc::SocketRunner + end opts.on("-d", "--[no-]debug", "Debug Mode. Override log level to DEBUG.") do |v| server.debug = v @@ -67,47 +66,37 @@ parser = OptionParser.new do |opts| end end +parser.parse! +require 'protobuf' + +[:INT, :QUIT, :TERM].each do |sig| + trap(sig) do + server.runner.stop + end +end + begin - parser.parse! - if ARGV.empty? raise 'You must specify an app file to use.' else server.app = ARGV.pop raise 'Invalid app file specified (%s).' % server.app unless File.exists?(server.app) end - + # Configure the Protobuf::Logger Protobuf::Logger.configure :file => server.log, :level => server.debug ? ::Logger::DEBUG : server.level # Output the server opts Protobuf::Logger.debug 'Debugging options:' Protobuf::Logger.debug server.inspect - - # Ensure errors thrown within EM are caught and logged appropriately - EventMachine.error_handler do |error| - if error.message == 'no acceptor' - raise 'Failed binding to %s:%d (%s)' % [server.host, server.port, error.message] - else - Protobuf::Logger.error error.message - Protobuf::Logger.error error.backtrace.join("\n") - end - end # Set the name of the process $0 = 'rpc_server %s:%d' % [server.host, server.port] # Require the given application file require server.app - - # Startup and run the rpc server - EM.schedule do - EventMachine.start_server(server.host, server.port, Protobuf::Rpc::Server) && \ - Protobuf::Logger.info('RPC Server listening at %s:%d in %s' % [server.host, server.port, server.env]) - end - # Join or start the reactor - EM.reactor_running? ? EM.reactor_thread.join : EM.run + server.runner.run(server) rescue msg = 'ERROR: RPC Server failed to start. %s' % $!.message $stderr.puts msg, *($!.backtrace) diff --git a/lib/protobuf.rb b/lib/protobuf.rb index dd935107..1f9b3e44 100644 --- a/lib/protobuf.rb +++ b/lib/protobuf.rb @@ -1,12 +1,18 @@ +require 'logger' require 'socket' require 'socketpool' -require 'eventmachine' -require 'protobuf/ext/eventmachine' module Protobuf end -require 'protobuf/rpc/client' -require 'protobuf/rpc/service' -require 'protobuf/rpc/servers/socket_server' -require 'protobuf/rpc/servers/evented_server' +if defined?(Protobuf::ServerType) && Protobuf::ServerType == "SocketServer" + require 'protobuf/rpc/client' + require 'protobuf/rpc/service' + require 'protobuf/rpc/servers/socket_server' +else + require 'eventmachine' + require 'protobuf/ext/eventmachine' + require 'protobuf/rpc/client' + require 'protobuf/rpc/service' + require 'protobuf/rpc/servers/evented_server' +end diff --git a/lib/protobuf/common/logger.rb b/lib/protobuf/common/logger.rb index d790c8d2..21ebbc59 100644 --- a/lib/protobuf/common/logger.rb +++ b/lib/protobuf/common/logger.rb @@ -1,5 +1,3 @@ -require 'logger' - module Protobuf class Logger < ::Logger @@ -61,4 +59,4 @@ module LogMethods end end -end \ No newline at end of file +end diff --git a/lib/protobuf/rpc/servers/evented_runner.rb b/lib/protobuf/rpc/servers/evented_runner.rb new file mode 100644 index 00000000..f637544b --- /dev/null +++ b/lib/protobuf/rpc/servers/evented_runner.rb @@ -0,0 +1,36 @@ +module Protobuf + module Rpc + class EventedRunner + + class << self + + def stop + EventMachine.stop_event_loop if EventMachine.reactor_running? + Protobuf::Logger.info 'Shutdown complete' + $stdout.puts 'Shutdown complete' + end + + def run(server) + # Ensure errors thrown within EM are caught and logged appropriately + EventMachine.error_handler do |error| + if error.message == 'no acceptor' + raise 'Failed binding to %s:%d (%s)' % [server.host, server.port, error.message] + else + Protobuf::Logger.error error.message + Protobuf::Logger.error error.backtrace.join("\n") + end + end + + # Startup and run the rpc server + EM.schedule do + EventMachine.start_server(server.host, server.port, Protobuf::Rpc::Server) && \ + Protobuf::Logger.info('RPC Server listening at %s:%d in %s' % [server.host, server.port, server.env]) + end + + # Join or start the reactor + EM.reactor_running? ? EM.reactor_thread.join : EM.run + end + end + end + end +end diff --git a/lib/protobuf/rpc/servers/socket_runner.rb b/lib/protobuf/rpc/servers/socket_runner.rb new file mode 100644 index 00000000..96b19895 --- /dev/null +++ b/lib/protobuf/rpc/servers/socket_runner.rb @@ -0,0 +1,22 @@ +module Protobuf + module Rpc + class SocketRunner + + class << self + + def stop + Protobuf::Rpc::SocketServer.stop + rescue + Protobuf::Logger.info 'Shutdown complete' + $stdout.puts 'Shutdown complete' + end + + def run(server) + Protobuf::Logger.info "SocketServer Running" + Protobuf::Rpc::SocketServer.run(server.host, server.port) if !Protobuf::Rpc::SocketServer.running? + end + + end + end + end +end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index d1ff20b0..ecffb285 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -7,17 +7,27 @@ class SocketServer include Protobuf::Logger::LogMethods class << self + def stop + @server.close + @running = false + end + def run(host = "127.0.0.1", port = 9399) - server = TCPServer.new(host, port) - server.listen(100) + @running = true + @server = TCPServer.new(host, port) + @server.listen(100) loop do - Thread.new(server.accept) do |sock| + Thread.new(@server.accept) do |sock| service_worker = self.new(sock) sock.close end end end + + def running? + defined?(@running) && @running + end end def initialize(sock) diff --git a/spec/unit/server_spec.rb b/spec/unit/evented_server_spec.rb similarity index 100% rename from spec/unit/server_spec.rb rename to spec/unit/evented_server_spec.rb diff --git a/spec/unit/socket_server_spec.rb b/spec/unit/socket_server_spec.rb new file mode 100644 index 00000000..c8da9be1 --- /dev/null +++ b/spec/unit/socket_server_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' +require 'spec/proto/test_service_impl' + +describe Protobuf::Rpc::Server do + context 'when sending response objects' do + it 'should be able to send a hash object as a response' do + server = Protobuf::Rpc::SocketServer.new + + # Setup the right mocks + server.instance_variable_set(:@klass, Spec::Proto::TestService) + response_wrapper = mock('response') + response_wrapper.stub(:response_proto=) + server.instance_variable_set(:@response, response_wrapper) + Spec::Proto::TestService.stub_chain(:rpcs, :[], :[], :response_type).and_return(Spec::Proto::ResourceFindRequest) + + # Setup expectations + hash_response = {:name => 'Test Name', :active => false} + expected = Spec::Proto::ResourceFindRequest.new(hash_response) + Spec::Proto::ResourceFindRequest.should_receive(:new).with(hash_response).and_return(expected) + server.should_not_receive(:handle_error) + + # Call the method + server.send(:parse_response_from_service, hash_response) + end + end +end From b51b2675c900213f972db96373e9d36b351ccc2a Mon Sep 17 00:00:00 2001 From: = Date: Fri, 2 Dec 2011 09:44:26 -0700 Subject: [PATCH 03/50] remove socketpool dependency --- lib/protobuf.rb | 1 - protobuf.gemspec | 1 - 2 files changed, 2 deletions(-) diff --git a/lib/protobuf.rb b/lib/protobuf.rb index 1f9b3e44..fd28eb7c 100644 --- a/lib/protobuf.rb +++ b/lib/protobuf.rb @@ -1,6 +1,5 @@ require 'logger' require 'socket' -require 'socketpool' module Protobuf end diff --git a/protobuf.gemspec b/protobuf.gemspec index f692df07..94e9ca62 100644 --- a/protobuf.gemspec +++ b/protobuf.gemspec @@ -19,7 +19,6 @@ Gem::Specification.new do |s| s.require_paths = ["lib"] s.add_dependency 'eventmachine', '~> 0.12.10' - s.add_dependency 'socketpool' s.add_development_dependency 'rake', '~> 0.8.7' s.add_development_dependency 'rspec', '~> 2.7.0' From 92e236f65f3270ce17e3766e5aa54774900e7804 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 2 Dec 2011 11:15:04 -0700 Subject: [PATCH 04/50] update gemlock --- Gemfile.lock | 3 --- 1 file changed, 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index dadb6216..0ac3b5e6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -3,14 +3,12 @@ PATH specs: protobuf (1.0.0) eventmachine (~> 0.12.10) - socketpool GEM remote: http://rubygems.org/ specs: diff-lcs (1.1.2) eventmachine (0.12.10) - eventmachine (0.12.10-java) rake (0.8.7) rspec (2.7.0) rspec-core (~> 2.7.0) @@ -20,7 +18,6 @@ GEM rspec-expectations (2.7.0) diff-lcs (~> 1.1.2) rspec-mocks (2.7.0) - socketpool (0.1.4) PLATFORMS java From 1989363d64e25de002b97a7c4f81e81865cb1ac3 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 2 Dec 2011 14:18:50 -0700 Subject: [PATCH 05/50] refactor connector to be constant based, add warning suppression in rspec --- lib/protobuf.rb | 13 +++++++ lib/protobuf/rpc/client.rb | 2 +- lib/protobuf/rpc/connector.rb | 16 +++++--- lib/protobuf/rpc/connectors/em_client.rb | 3 +- lib/protobuf/rpc/connectors/eventmachine.rb | 2 +- spec/helper/all.rb | 8 +--- spec/helper/server.rb | 2 +- spec/helper/silent_constants.rb | 41 +++++++++++++++++++++ spec/spec_helper.rb | 16 +++++--- spec/unit/rpc/client_spec.rb | 1 - spec/unit/rpc/connector_spec.rb | 27 ++++++-------- 11 files changed, 90 insertions(+), 41 deletions(-) create mode 100644 spec/helper/silent_constants.rb diff --git a/lib/protobuf.rb b/lib/protobuf.rb index fd28eb7c..9ef535f6 100644 --- a/lib/protobuf.rb +++ b/lib/protobuf.rb @@ -4,6 +4,7 @@ module Protobuf end +# For running the rpc_server if defined?(Protobuf::ServerType) && Protobuf::ServerType == "SocketServer" require 'protobuf/rpc/client' require 'protobuf/rpc/service' @@ -15,3 +16,15 @@ module Protobuf require 'protobuf/rpc/service' require 'protobuf/rpc/servers/evented_server' end + +# When setting up a client +if defined?(Protobuf::ConnectorType) && Protobuf::ConnectorType == "Socket" + require 'protobuf/rpc/client' + require 'protobuf/rpc/connectors/socket' +else + Protobuf::ConnectorType = "EventMachine" + require 'eventmachine' + require 'protobuf/ext/eventmachine' + require 'protobuf/rpc/client' + require 'protobuf/rpc/connectors/eventmachine' +end diff --git a/lib/protobuf/rpc/client.rb b/lib/protobuf/rpc/client.rb index a4aa9947..2761f97d 100644 --- a/lib/protobuf/rpc/client.rb +++ b/lib/protobuf/rpc/client.rb @@ -27,7 +27,7 @@ class Client # def initialize opts={} raise "Invalid client configuration. Service must be defined." if !opts[:service] || opts[:service].nil? - @connector = Connector.connector_for_platform.new(opts) + @connector = Connector.connector_for_client.new(opts) log_debug '[client] Initialized with options: %s' % opts.inspect end diff --git a/lib/protobuf/rpc/connector.rb b/lib/protobuf/rpc/connector.rb index 752e8603..8108026f 100644 --- a/lib/protobuf/rpc/connector.rb +++ b/lib/protobuf/rpc/connector.rb @@ -5,15 +5,19 @@ module Protobuf module Rpc class Connector - def self.connector_for_platform platform=RUBY_ENGINE - case platform - when /jruby/i - Connectors::Socket + def self.connector_for_client + if defined?(Protobuf::ConnectorType) + case Protobuf::ConnectorType + when "Socket" then + ::Protobuf::Rpc::Connectors::Socket + else + ::Protobuf::Rpc::Connectors::EventMachine + end else - Connectors::EventMachine + ::Protobuf::Rpc::Connectors::EventMachine end end end end -end \ No newline at end of file +end diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index be0c1180..36cbb885 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -1,4 +1,3 @@ -require 'eventmachine' require 'protobuf/common/logger' require 'protobuf/rpc/rpc.pb' require 'protobuf/rpc/buffer' @@ -224,4 +223,4 @@ def complete end end end -end \ No newline at end of file +end diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index 3cc96913..fa6ad68f 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -81,4 +81,4 @@ def resume_fiber(fib) end end end -end \ No newline at end of file +end diff --git a/spec/helper/all.rb b/spec/helper/all.rb index f059da27..802cc96a 100644 --- a/spec/helper/all.rb +++ b/spec/helper/all.rb @@ -1,13 +1,7 @@ -require 'rubygems' -require 'rspec' - require 'helper/tolerance_matcher' require 'helper/server' +require 'helper/silent_constants' def now Time.new.to_f end - -RSpec.configure do |con| - con.include(Sander6::CustomMatchers) -end diff --git a/spec/helper/server.rb b/spec/helper/server.rb index a0d7a8e4..944bac84 100644 --- a/spec/helper/server.rb +++ b/spec/helper/server.rb @@ -1,4 +1,4 @@ -require 'lib/protobuf/rpc/server' +require 'protobuf/rpc/server' require 'spec/proto/test_service_impl' module StubProtobufServerFactory diff --git a/spec/helper/silent_constants.rb b/spec/helper/silent_constants.rb new file mode 100644 index 00000000..c959f6ed --- /dev/null +++ b/spec/helper/silent_constants.rb @@ -0,0 +1,41 @@ +# Just a way to keep warnings from being flagged in rename of constants during tests +module Kernel + def silence_warnings + orig_verbosity = $VERBOSE + $VERBOSE = nil + yield + $VERBOSE = orig_verbosity + end +end + +module SilentConstants + def parse(constant) + source, _, constant_name = constant.to_s.rpartition('::') + [Object.const_get(source.to_sym), constant_name.to_sym] + end + + # Examples + # with_constants "Something" => "nothing" do + # end + # + # with_constants "Something::Foo" => "something else" do + # end + # + def with_constants(constants, &block) + saved_constants = {} + constants.each do |constant, val| + source_object, const_name = parse(constant) + saved_constants[constant] = source_object.const_get(const_name) + Kernel::silence_warnings { source_object.const_set(const_name, val) } + end + + begin + block.call + ensure + constants.each do |constant, val| + source_object, const_name = parse(constant) + Kernel::silence_warnings { source_object.const_set(const_name, saved_constants[constant]) } + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b1716e25..338a4967 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,15 +2,19 @@ require 'bundler' Bundler.setup :default, :development, :test -RSpec.configure do |c| - c.mock_with :rspec -end - $:.push File.expand_path('..', File.dirname(__FILE__)) $:.push File.expand_path('../lib', File.dirname(__FILE__)) -require 'protobuf' +require 'protobuf' require 'protobuf/rpc/client' +require File.dirname(__FILE__) + '/helper/all' + +RSpec.configure do |c| + c.include(SilentConstants) + c.include(Sander6::CustomMatchers) + c.mock_with :rspec +end + class ::Protobuf::Rpc::Client def == other connector.options == other.options && \ @@ -21,4 +25,4 @@ def == other def reset_service_location service service.instance_variable_set :@locations, nil -end \ No newline at end of file +end diff --git a/spec/unit/rpc/client_spec.rb b/spec/unit/rpc/client_spec.rb index 50cd9389..7b97ee3e 100644 --- a/spec/unit/rpc/client_spec.rb +++ b/spec/unit/rpc/client_spec.rb @@ -1,6 +1,5 @@ require 'spec_helper' require 'spec/proto/test_service_impl' -require 'spec/helper/all' describe Protobuf::Rpc::Client do diff --git a/spec/unit/rpc/connector_spec.rb b/spec/unit/rpc/connector_spec.rb index 00b7d1f7..d29afd68 100644 --- a/spec/unit/rpc/connector_spec.rb +++ b/spec/unit/rpc/connector_spec.rb @@ -2,32 +2,27 @@ describe Protobuf::Rpc::Connector do - describe '.connector_for_platform' do + describe '.connector_for_client' do - context 'when platform is java' do - let(:platform) { 'jruby' } + context 'when set to Socket connector' do it 'returns a socket connector class reference' do - Protobuf::Rpc::Connector.connector_for_platform(platform).should eq Protobuf::Rpc::Connectors::Socket + with_constants "Protobuf::ConnectorType" => "Socket" do + Protobuf::Rpc::Connector.connector_for_client.should eq(Protobuf::Rpc::Connectors::Socket) + end end end - context 'when platform is mri' do - let(:platform) { 'mri' } + context 'when set to non Socket Connector' do it 'returns an eventmachine connector class reference' do - Protobuf::Rpc::Connector.connector_for_platform(platform).should eq Protobuf::Rpc::Connectors::EventMachine + with_constants "Protobuf::ConnectorType" => "EventMachine" do + Protobuf::Rpc::Connector.connector_for_client.should eq Protobuf::Rpc::Connectors::EventMachine + end end end - context 'when platform is unknown' do - let(:platform) { 'some_bogus_engine' } + context 'when connector type not given' do it 'returns an eventmachine connector class reference' do - Protobuf::Rpc::Connector.connector_for_platform(platform).should eq Protobuf::Rpc::Connectors::EventMachine - end - end - - context 'when platform is not given' do - it 'returns an eventmachine connector class reference' do - Protobuf::Rpc::Connector.connector_for_platform.should eq Protobuf::Rpc::Connectors::EventMachine + Protobuf::Rpc::Connector.connector_for_client.should eq Protobuf::Rpc::Connectors::EventMachine end end From d6a832edeffcff44bbfd54a7f1c1a66553b1f2d5 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 2 Dec 2011 14:59:39 -0700 Subject: [PATCH 06/50] clean up the rpc server log by catching the EBADF on server socket close --- lib/protobuf.rb | 24 +++++++++++------------ lib/protobuf/rpc/servers/socket_runner.rb | 3 +-- lib/protobuf/rpc/servers/socket_server.rb | 8 +++++++- spec/proto/test_service.rb | 2 +- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/lib/protobuf.rb b/lib/protobuf.rb index 9ef535f6..a265141f 100644 --- a/lib/protobuf.rb +++ b/lib/protobuf.rb @@ -4,19 +4,6 @@ module Protobuf end -# For running the rpc_server -if defined?(Protobuf::ServerType) && Protobuf::ServerType == "SocketServer" - require 'protobuf/rpc/client' - require 'protobuf/rpc/service' - require 'protobuf/rpc/servers/socket_server' -else - require 'eventmachine' - require 'protobuf/ext/eventmachine' - require 'protobuf/rpc/client' - require 'protobuf/rpc/service' - require 'protobuf/rpc/servers/evented_server' -end - # When setting up a client if defined?(Protobuf::ConnectorType) && Protobuf::ConnectorType == "Socket" require 'protobuf/rpc/client' @@ -28,3 +15,14 @@ module Protobuf require 'protobuf/rpc/client' require 'protobuf/rpc/connectors/eventmachine' end + +# For running the rpc_server +if defined?(Protobuf::ServerType) && Protobuf::ServerType == "SocketServer" + require 'protobuf/rpc/service' + require 'protobuf/rpc/servers/socket_server' +else + require 'eventmachine' + require 'protobuf/ext/eventmachine' + require 'protobuf/rpc/service' + require 'protobuf/rpc/servers/evented_server' +end diff --git a/lib/protobuf/rpc/servers/socket_runner.rb b/lib/protobuf/rpc/servers/socket_runner.rb index 96b19895..4e02cc01 100644 --- a/lib/protobuf/rpc/servers/socket_runner.rb +++ b/lib/protobuf/rpc/servers/socket_runner.rb @@ -5,8 +5,7 @@ class SocketRunner class << self def stop - Protobuf::Rpc::SocketServer.stop - rescue + Protobuf::Rpc::SocketServer.stop Protobuf::Logger.info 'Shutdown complete' $stdout.puts 'Shutdown complete' end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index ecffb285..f74a32cb 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -8,8 +8,8 @@ class SocketServer class << self def stop - @server.close @running = false + @server.close end def run(host = "127.0.0.1", port = 9399) @@ -23,6 +23,12 @@ def run(host = "127.0.0.1", port = 9399) sock.close end end + + rescue Errno::EBADF + # Closing the server causes the loop to raise an exception here + if running? + raise + end end def running? diff --git a/spec/proto/test_service.rb b/spec/proto/test_service.rb index 02699f00..cda40342 100644 --- a/spec/proto/test_service.rb +++ b/spec/proto/test_service.rb @@ -1,5 +1,5 @@ require 'protobuf/rpc/service' -require 'spec/proto/test.pb' +require File.dirname(__FILE__) + '/test.pb' ## !! DO NOT EDIT THIS FILE !! ## From caa020a7cd681363db2a6bff3cc1452c33979e47 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 3 Dec 2011 11:29:31 -0700 Subject: [PATCH 07/50] just spacing stuff --- lib/protobuf/rpc/connectors/eventmachine.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index fa6ad68f..1f2e934f 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -6,7 +6,7 @@ module Rpc module Connectors class EventMachine < Base - def initialize options + def initialize(options) super(EMClient::DEFAULT_OPTIONS.merge(options)) end @@ -77,7 +77,6 @@ def resume_fiber(fib) ensure_cb.call(err) end - end end end From 0a9a90894db3b2da9395d676accba517cc424cde Mon Sep 17 00:00:00 2001 From: = Date: Sat, 3 Dec 2011 12:53:41 -0700 Subject: [PATCH 08/50] factor out some superfluous code on clients and org for socket client --- lib/protobuf/rpc/connectors/base.rb | 20 +++- lib/protobuf/rpc/connectors/em_client.rb | 126 +++++++++----------- lib/protobuf/rpc/connectors/eventmachine.rb | 12 +- lib/protobuf/rpc/connectors/socket.rb | 12 ++ lib/protobuf/rpc/stat.rb | 2 +- spec/unit/rpc/connectors/base_spec.rb | 2 +- 6 files changed, 92 insertions(+), 82 deletions(-) diff --git a/lib/protobuf/rpc/connectors/base.rb b/lib/protobuf/rpc/connectors/base.rb index 983cda49..b9f7e3a0 100644 --- a/lib/protobuf/rpc/connectors/base.rb +++ b/lib/protobuf/rpc/connectors/base.rb @@ -3,14 +3,26 @@ module Protobuf module Rpc module Connectors + DEFAULT_OPTIONS = { + :service => nil, # Service to invoke + :method => nil, # Service method to call + :host => 'localhost', # A default host (usually overridden) + :port => '9938', # A default port (usually overridden) + :request => nil, # The request object sent by the client + :request_type => nil, # The request type expected by the client + :response_type => nil, # The response type expected by the client + :async => false, # Whether or not to block a client call, this is actually handled by client.rb + :timeout => 30 # The default timeout for the request, also handled by client.rb + } + class Base include Protobuf::Logger::LogMethods attr_reader :options attr_accessor :success_cb, :failure_cb - - def initialize options - @options = options + + def initialize(options) + @options = DEFAULT_OPTIONS.merge(options) @success_cb = nil @failure_cb = nil end @@ -26,4 +38,4 @@ def async? end end end -end \ No newline at end of file +end diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index 36cbb885..8c9afdf9 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -15,18 +15,6 @@ class EMClient < EM::Connection attr_reader :options, :request, :response attr_reader :error, :error_reason, :error_message - - DEFAULT_OPTIONS = { - :service => nil, # Service to invoke - :method => nil, # Service method to call - :host => 'localhost', # A default host (usually overridden) - :port => '9938', # A default port (usually overridden) - :request => nil, # The request object sent by the client - :request_type => nil, # The request type expected by the client - :response_type => nil, # The response type expected by the client - :async => false, # Whether or not to block a client call, this is actually handled by client.rb - :timeout => 30 # The default timeout for the request, also handled by client.rb - } # For state tracking STATES = { @@ -36,48 +24,43 @@ class EMClient < EM::Connection :completed => 3 } - def self.connect options={} - options = DEFAULT_OPTIONS.merge(options) - Protobuf::Logger.debug '[client-cnxn] Connecting to server: %s' % options.inspect - host = options[:host] - port = options[:port] - EM.connect host, port, self, options + class << self + + def connect(options={}) + options = DEFAULT_OPTIONS.merge(options) + Protobuf::Logger.debug '[client-cnxn] Connecting to server: %s' % options.inspect + EM.connect(options[:host], options[:port], self, options) + end + end - def initialize options={}, &failure_cb + def initialize(options={}, &failure_cb) @failure_cb = failure_cb - - # Verify the options that are necessary and merge them in - [:service, :method, :host, :port].each do |opt| - fail(:RPC_ERROR, "Invalid client connection configuration. #{opt} must be a defined option.") if !options[opt] || options[opt].nil? - end @options = DEFAULT_OPTIONS.merge(options) + verify_options + log_debug '[client-cnxn] Client Initialized: %s' % options.inspect - @error = ClientError.new @success_cb = nil @state = STATES[:pending] - - @stats = Protobuf::Rpc::Stat.new(:CLIENT, true) - @stats.server = [@options[:port], @options[:host]] - @stats.service = @options[:service].name - @stats.method = @options[:method] + + initialize_stats rescue fail(:RPC_ERROR, 'Failed to initialize connection: %s' % $!.message) unless failed? end # Success callback registration - def on_success &success_cb + def on_success(&success_cb) @success_cb = success_cb end # Failure callback registration - def on_failure &failure_cb + def on_failure(&failure_cb) @failure_cb = failure_cb end # Completion callback registration - def on_complete &complete_cb + def on_complete(&complete_cb) @complete_cb = complete_cb end @@ -97,7 +80,7 @@ def post_init fail(:RPC_ERROR, 'Connection error: %s' % $!.message) unless failed? end - def receive_data data + def receive_data(data) log_debug '[client-cnxn] receive_data: %s' % data @buffer << data parse_response if @buffer.flushed? @@ -120,6 +103,20 @@ def completed? end private + + def initialize_stats + @stats = Protobuf::Rpc::Stat.new(:CLIENT, true) + @stats.server = [@options[:port], @options[:host]] + @stats.service = @options[:service].name + @stats.method = @options[:method] + end + + def verify_options + # Verify the options that are necessary and merge them in + [:service, :method, :host, :port].each do |opt| + fail(:RPC_ERROR, "Invalid client connection configuration. #{opt} must be a defined option.") if @options[opt].nil? + end + end # Sends the request to the server, invoked by the connection_completed event def send_request @@ -169,55 +166,48 @@ def parse_response if parsed.nil? and not response_wrapper.has_field?(:error_reason) fail :BAD_RESPONSE_PROTO, 'Unable to parse response from server' else - succeed parsed + succeed(parsed) end end end - def fail code, message + def fail(code, message) @state = STATES[:failed] @error.code = code.is_a?(Symbol) ? Protobuf::Socketrpc::ErrorReason.values[code] : code @error.message = message log_debug '[client-cnxn] Server failed request (invoking on_failure): %s' % @error.inspect - begin - @stats.end - @stats.log_stats - @failure_cb.call(@error) unless @failure_cb.nil? - rescue - log_error '[client-cnxn] Failure callback error encountered: %s' % $!.message - log_error '[client-cnxn] %s' % $!.backtrace.join("\n") - raise - ensure - complete - end + @failure_cb.call(@error) unless @failure_cb.nil? + rescue + log_error '[client-cnxn] Failure callback error encountered: %s' % $!.message + log_error '[client-cnxn] %s' % $!.backtrace.join("\n") + raise + ensure + complete end - def succeed response + def succeed(response) @state = STATES[:succeeded] - begin - log_debug '[client-cnxn] Server succeeded request (invoking on_success)' - @stats.end - @stats.log_stats - @success_cb.call(response) unless @success_cb.nil? - complete - rescue - log_error '[client-cnxn] Success callback error encountered: %s' % $!.message - log_error '[client-cnxn] %s' % $!.backtrace.join("\n") - fail :RPC_ERROR, 'An exception occurred while calling on_success: %s' % $!.message - end + log_debug '[client-cnxn] Server succeeded request (invoking on_success)' + @success_cb.call(response) unless @success_cb.nil? + rescue + log_error '[client-cnxn] Success callback error encountered: %s' % $!.message + log_error '[client-cnxn] %s' % $!.backtrace.join("\n") + fail :RPC_ERROR, 'An exception occurred while calling on_success: %s' % $!.message + ensure + complete end def complete @state = STATES[:completed] - begin - log_debug '[client-cnxn] Response proceessing complete' - @success_cb = @failure_cb = nil - @complete_cb.call(@state) unless @complete_cb.nil? - rescue - log_error '[client-cnxn] Complete callback error encountered: %s' % $!.message - log_error '[client-cnxn] %s' % $!.backtrace.join("\n") - raise - end + @stats.end + @stats.log_stats + log_debug '[client-cnxn] Response proceessing complete' + @success_cb = @failure_cb = nil + @complete_cb.call(@state) unless @complete_cb.nil? + rescue + log_error '[client-cnxn] Complete callback error encountered: %s' % $!.message + log_error '[client-cnxn] %s' % $!.backtrace.join("\n") + raise end end diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index 1f2e934f..b5af8b72 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -6,10 +6,6 @@ module Rpc module Connectors class EventMachine < Base - def initialize(options) - super(EMClient::DEFAULT_OPTIONS.merge(options)) - end - def send_request Thread.new { EM.run } unless EM.reactor_running? @@ -17,10 +13,10 @@ def send_request EM.schedule do log_debug '[client] Scheduling EventMachine client request to be created on next tick' - cnxn = EMClient.connect options, &ensure_cb - cnxn.on_success &success_cb if success_cb - cnxn.on_failure &ensure_cb - cnxn.on_complete { resume_fiber f } unless async? + cnxn = EMClient.connect(options, &ensure_cb) + cnxn.on_success(&success_cb) if success_cb + cnxn.on_failure(&ensure_cb) + cnxn.on_complete { resume_fiber(f) } unless async? log_debug '[client] Connection scheduled' end diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index 3ad5540e..a2d57a5e 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -6,6 +6,18 @@ module Connectors class Socket < Base def send_request + check_async + connect_to_rpc_server + end + + private + + def check_async + raise "Cannot use Socket client in async mode" if async? + end + + def connect_to_rpc_server + end end diff --git a/lib/protobuf/rpc/stat.rb b/lib/protobuf/rpc/stat.rb index 9188ea35..6314dedf 100644 --- a/lib/protobuf/rpc/stat.rb +++ b/lib/protobuf/rpc/stat.rb @@ -51,7 +51,7 @@ def elapsed_time end def log_stats - Protobuf::Logger.info to_s + Protobuf::Logger.info(self.to_s) end def to_s diff --git a/spec/unit/rpc/connectors/base_spec.rb b/spec/unit/rpc/connectors/base_spec.rb index 6553f7f6..1ef3e69d 100644 --- a/spec/unit/rpc/connectors/base_spec.rb +++ b/spec/unit/rpc/connectors/base_spec.rb @@ -10,7 +10,7 @@ describe '.new' do it 'assigns passed options and initializes success/failure callbacks' do - subject.options.should eq opts + subject.options.should eq(Protobuf::Rpc::Connectors::DEFAULT_OPTIONS.merge(opts)) subject.success_cb.should be_nil subject.failure_cb.should be_nil end From 965e8ff799a1cd373b09cad2f18b65703ece71c2 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 3 Dec 2011 15:58:18 -0700 Subject: [PATCH 09/50] spacing/preferences --- lib/protobuf/rpc/client.rb | 10 +++++----- lib/protobuf/rpc/connectors/base.rb | 4 +--- lib/protobuf/rpc/connectors/em_client.rb | 13 ++++++++----- lib/protobuf/rpc/connectors/socket.rb | 3 ++- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/protobuf/rpc/client.rb b/lib/protobuf/rpc/client.rb index 2761f97d..1a544ee8 100644 --- a/lib/protobuf/rpc/client.rb +++ b/lib/protobuf/rpc/client.rb @@ -26,7 +26,7 @@ class Client # }) # def initialize opts={} - raise "Invalid client configuration. Service must be defined." if !opts[:service] || opts[:service].nil? + raise "Invalid client configuration. Service must be defined." if opts[:service].nil? @connector = Connector.connector_for_client.new(opts) log_debug '[client] Initialized with options: %s' % opts.inspect end @@ -39,7 +39,7 @@ def initialize opts={} # client = Client.new(:service => WidgetService) # client.on_success {|res| ... } # - def on_success &success_cb + def on_success(&success_cb) @connector.success_cb = success_cb end @@ -51,7 +51,7 @@ def on_success &success_cb # client = Client.new(:service => WidgetService) # client.on_failure {|err| ... } # - def on_failure &failure_cb + def on_failure(&failure_cb) @connector.failure_cb = failure_cb end @@ -67,7 +67,7 @@ def on_failure &failure_cb # c.on_failure {|err| ... } # end # - def method_missing method, *params + def method_missing(method, *params) service = options[:service] unless service.rpcs[service].keys.include?(method) log_error '[client] %s#%s not rpc method, passing to super' % [service.name, method.to_s] @@ -88,7 +88,7 @@ def method_missing method, *params log_debug '[client] client setup callback given, invoking' yield(self) else - log_debug '[client] no callbacks given' + log_debug '[client] no block given for callbacks' end send_request diff --git a/lib/protobuf/rpc/connectors/base.rb b/lib/protobuf/rpc/connectors/base.rb index b9f7e3a0..2cc7d609 100644 --- a/lib/protobuf/rpc/connectors/base.rb +++ b/lib/protobuf/rpc/connectors/base.rb @@ -23,12 +23,10 @@ class Base def initialize(options) @options = DEFAULT_OPTIONS.merge(options) - @success_cb = nil - @failure_cb = nil end def send_request - raise 'not implemented' + raise 'If you inherit a Connector from Base you must implement send_request' end def async? diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index 8c9afdf9..373bc8b3 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -40,7 +40,6 @@ def initialize(options={}, &failure_cb) verify_options log_debug '[client-cnxn] Client Initialized: %s' % options.inspect - @error = ClientError.new @success_cb = nil @state = STATES[:pending] @@ -49,6 +48,10 @@ def initialize(options={}, &failure_cb) fail(:RPC_ERROR, 'Failed to initialize connection: %s' % $!.message) unless failed? end + def error + @error || ClientError.new + end + # Success callback registration def on_success(&success_cb) @success_cb = success_cb @@ -173,10 +176,10 @@ def parse_response def fail(code, message) @state = STATES[:failed] - @error.code = code.is_a?(Symbol) ? Protobuf::Socketrpc::ErrorReason.values[code] : code - @error.message = message - log_debug '[client-cnxn] Server failed request (invoking on_failure): %s' % @error.inspect - @failure_cb.call(@error) unless @failure_cb.nil? + error.code = code.is_a?(Symbol) ? Protobuf::Socketrpc::ErrorReason.values[code] : code + error.message = message + log_debug '[client-cnxn] Server failed request (invoking on_failure): %s' % error.inspect + @failure_cb.call(error) unless @failure_cb.nil? rescue log_error '[client-cnxn] Failure callback error encountered: %s' % $!.message log_error '[client-cnxn] %s' % $!.backtrace.join("\n") diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index a2d57a5e..4b8a5e6b 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -17,7 +17,8 @@ def check_async end def connect_to_rpc_server - + @socket = TCPSocket.new(options[:host], options[:port]) + end end From 61c0342bd1942cb08b49a1ac8a62a98898750327 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 3 Dec 2011 16:32:17 -0700 Subject: [PATCH 10/50] move common methods for connectors into module --- lib/protobuf/rpc/connectors/base.rb | 5 + lib/protobuf/rpc/connectors/common.rb | 130 ++++++++++++++++++++++ lib/protobuf/rpc/connectors/em_client.rb | 131 +---------------------- lib/protobuf/rpc/connectors/socket.rb | 8 +- 4 files changed, 142 insertions(+), 132 deletions(-) create mode 100644 lib/protobuf/rpc/connectors/common.rb diff --git a/lib/protobuf/rpc/connectors/base.rb b/lib/protobuf/rpc/connectors/base.rb index 2cc7d609..15720eb5 100644 --- a/lib/protobuf/rpc/connectors/base.rb +++ b/lib/protobuf/rpc/connectors/base.rb @@ -1,4 +1,9 @@ require 'protobuf/common/logger' +require 'protobuf/rpc/rpc.pb' +require 'protobuf/rpc/buffer' +require 'protobuf/rpc/error' +require 'protobuf/rpc/stat' +require 'protobuf/rpc/connectors/common' module Protobuf module Rpc diff --git a/lib/protobuf/rpc/connectors/common.rb b/lib/protobuf/rpc/connectors/common.rb new file mode 100644 index 00000000..2f6531b3 --- /dev/null +++ b/lib/protobuf/rpc/connectors/common.rb @@ -0,0 +1,130 @@ +module Protobuf + module Rpc + module Connectors + module Common + # For state tracking + STATES = { + :pending => 0, + :succeeded => 1, + :failed => 2, + :completed => 3 + } + + def complete + @state = STATES[:completed] + @stats.end + @stats.log_stats + log_debug '[client-cnxn] Response proceessing complete' + @success_cb = @failure_cb = nil + @complete_cb.call(@state) unless @complete_cb.nil? + rescue + log_error '[client-cnxn] Complete callback error encountered: %s' % $!.message + log_error '[client-cnxn] %s' % $!.backtrace.join("\n") + raise + end + + def fail(code, message) + @state = STATES[:failed] + error.code = code.is_a?(Symbol) ? Protobuf::Socketrpc::ErrorReason.values[code] : code + error.message = message + log_debug '[client-cnxn] Server failed request (invoking on_failure): %s' % error.inspect + @failure_cb.call(error) unless @failure_cb.nil? + rescue + log_error '[client-cnxn] Failure callback error encountered: %s' % $!.message + log_error '[client-cnxn] %s' % $!.backtrace.join("\n") + raise + ensure + complete + end + + def initialize_stats + @stats = Protobuf::Rpc::Stat.new(:CLIENT, true) + @stats.server = [@options[:port], @options[:host]] + @stats.service = @options[:service].name + @stats.method = @options[:method] + end + + def parse_response + # Close up the connection as we no longer need it + close_connection + + log_debug '[client-cnxn] Parsing response from server (connection closed)' + @stats.response_size = @buffer.size + + # Parse out the raw response + response_wrapper = Protobuf::Socketrpc::Response.new + response_wrapper.parse_from_string(@buffer.data) + + # Determine success or failure based on parsed data + if response_wrapper.has_field? :error_reason + log_debug '[client-cnxn] Error response parsed' + + # fail the call if we already know the client is failed + # (don't try to parse out the response payload) + fail response_wrapper.error_reason, response_wrapper.error + else + log_debug '[client-cnxn] Successful response parsed' + + # Ensure client_response is an instance + response_type = @options[:response_type].new + parsed = response_type.parse_from_string(response_wrapper.response_proto.to_s) + + if parsed.nil? and not response_wrapper.has_field?(:error_reason) + fail :BAD_RESPONSE_PROTO, 'Unable to parse response from server' + else + succeed(parsed) + end + end + end + + # Setup the read buffer for data coming back + def post_init + log_debug '[client-cnxn] Post init, new read buffer created' + @buffer = Protobuf::Rpc::Buffer.new :read + rescue + fail(:RPC_ERROR, 'Connection error: %s' % $!.message) unless failed? + end + + # Sends the request to the server, invoked by the connection_completed event + def send_request + request_wrapper = Protobuf::Socketrpc::Request.new + request_wrapper.service_name = @options[:service].name + request_wrapper.method_name = @options[:method].to_s + + if @options[:request].class == @options[:request_type] + request_wrapper.request_proto = @options[:request].serialize_to_string + else + expected = @options[:request_type].name + actual = @options[:request].class.name + fail :INVALID_REQUEST_PROTO, 'Expected request type to be type of %s, got %s instead' % [expected, actual] + end + + log_debug '[client-cnxn] Sending Request: %s' % request_wrapper.inspect + request_buffer = Protobuf::Rpc::Buffer.new(:write, request_wrapper) + send_data(request_buffer.write) + @stats.request_size = request_buffer.size + end + + def succeed(response) + @state = STATES[:succeeded] + log_debug '[client-cnxn] Server succeeded request (invoking on_success)' + @success_cb.call(response) unless @success_cb.nil? + rescue + log_error '[client-cnxn] Success callback error encountered: %s' % $!.message + log_error '[client-cnxn] %s' % $!.backtrace.join("\n") + fail :RPC_ERROR, 'An exception occurred while calling on_success: %s' % $!.message + ensure + complete + end + + def verify_options + # Verify the options that are necessary and merge them in + [:service, :method, :host, :port].each do |opt| + fail(:RPC_ERROR, "Invalid client connection configuration. #{opt} must be a defined option.") if @options[opt].nil? + end + end + + end + end + end +end diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index 373bc8b3..1b9a5ea0 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -1,9 +1,3 @@ -require 'protobuf/common/logger' -require 'protobuf/rpc/rpc.pb' -require 'protobuf/rpc/buffer' -require 'protobuf/rpc/error' -require 'protobuf/rpc/stat' - # Handles client connections to the server module Protobuf module Rpc @@ -12,18 +6,11 @@ module Connectors class EMClient < EM::Connection include Protobuf::Logger::LogMethods + include Protobuf::Rpc::Connectors::Common attr_reader :options, :request, :response attr_reader :error, :error_reason, :error_message - # For state tracking - STATES = { - :pending => 0, - :succeeded => 1, - :failed => 2, - :completed => 3 - } - class << self def connect(options={}) @@ -75,14 +62,6 @@ def connection_completed fail(:RPC_ERROR, 'Connection error: %s' % $!.message) unless failed? end - # Setup the read buffer for data coming back - def post_init - log_debug '[client-cnxn] Post init, new read buffer created' - @buffer = Protobuf::Rpc::Buffer.new :read - rescue - fail(:RPC_ERROR, 'Connection error: %s' % $!.message) unless failed? - end - def receive_data(data) log_debug '[client-cnxn] receive_data: %s' % data @buffer << data @@ -105,114 +84,6 @@ def completed? @state == STATES[:completed] end - private - - def initialize_stats - @stats = Protobuf::Rpc::Stat.new(:CLIENT, true) - @stats.server = [@options[:port], @options[:host]] - @stats.service = @options[:service].name - @stats.method = @options[:method] - end - - def verify_options - # Verify the options that are necessary and merge them in - [:service, :method, :host, :port].each do |opt| - fail(:RPC_ERROR, "Invalid client connection configuration. #{opt} must be a defined option.") if @options[opt].nil? - end - end - - # Sends the request to the server, invoked by the connection_completed event - def send_request - request_wrapper = Protobuf::Socketrpc::Request.new - request_wrapper.service_name = @options[:service].name - request_wrapper.method_name = @options[:method].to_s - - if @options[:request].class == @options[:request_type] - request_wrapper.request_proto = @options[:request].serialize_to_string - else - expected = @options[:request_type].name - actual = @options[:request].class.name - fail :INVALID_REQUEST_PROTO, 'Expected request type to be type of %s, got %s instead' % [expected, actual] - end - - log_debug '[client-cnxn] Sending Request: %s' % request_wrapper.inspect - request_buffer = Protobuf::Rpc::Buffer.new(:write, request_wrapper) - send_data(request_buffer.write) - @stats.request_size = request_buffer.size - end - - def parse_response - # Close up the connection as we no longer need it - close_connection - - log_debug '[client-cnxn] Parsing response from server (connection closed)' - @stats.response_size = @buffer.size - - # Parse out the raw response - response_wrapper = Protobuf::Socketrpc::Response.new - response_wrapper.parse_from_string @buffer.data - - # Determine success or failure based on parsed data - if response_wrapper.has_field? :error_reason - log_debug '[client-cnxn] Error response parsed' - - # fail the call if we already know the client is failed - # (don't try to parse out the response payload) - fail response_wrapper.error_reason, response_wrapper.error - else - log_debug '[client-cnxn] Successful response parsed' - - # Ensure client_response is an instance - response_type = @options[:response_type].new - parsed = response_type.parse_from_string(response_wrapper.response_proto.to_s) - - if parsed.nil? and not response_wrapper.has_field?(:error_reason) - fail :BAD_RESPONSE_PROTO, 'Unable to parse response from server' - else - succeed(parsed) - end - end - end - - def fail(code, message) - @state = STATES[:failed] - error.code = code.is_a?(Symbol) ? Protobuf::Socketrpc::ErrorReason.values[code] : code - error.message = message - log_debug '[client-cnxn] Server failed request (invoking on_failure): %s' % error.inspect - @failure_cb.call(error) unless @failure_cb.nil? - rescue - log_error '[client-cnxn] Failure callback error encountered: %s' % $!.message - log_error '[client-cnxn] %s' % $!.backtrace.join("\n") - raise - ensure - complete - end - - def succeed(response) - @state = STATES[:succeeded] - log_debug '[client-cnxn] Server succeeded request (invoking on_success)' - @success_cb.call(response) unless @success_cb.nil? - rescue - log_error '[client-cnxn] Success callback error encountered: %s' % $!.message - log_error '[client-cnxn] %s' % $!.backtrace.join("\n") - fail :RPC_ERROR, 'An exception occurred while calling on_success: %s' % $!.message - ensure - complete - end - - def complete - @state = STATES[:completed] - @stats.end - @stats.log_stats - log_debug '[client-cnxn] Response proceessing complete' - @success_cb = @failure_cb = nil - @complete_cb.call(@state) unless @complete_cb.nil? - rescue - log_error '[client-cnxn] Complete callback error encountered: %s' % $!.message - log_error '[client-cnxn] %s' % $!.backtrace.join("\n") - raise - end - end end end diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index 4b8a5e6b..13e1dba4 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -4,23 +4,27 @@ module Protobuf module Rpc module Connectors class Socket < Base + include Protobuf::Rpc::Connectors::Common def send_request + post_init check_async connect_to_rpc_server end private + def send_data(data) + end + def check_async raise "Cannot use Socket client in async mode" if async? end def connect_to_rpc_server @socket = TCPSocket.new(options[:host], options[:port]) - end - + end end end From 03b9966cd9b8a2e95a4e86c46102e0fdd1091c6a Mon Sep 17 00:00:00 2001 From: = Date: Sat, 3 Dec 2011 18:24:08 -0700 Subject: [PATCH 11/50] first cut at socket server client --- lib/protobuf/rpc/buffer.rb | 10 +++++----- lib/protobuf/rpc/connectors/common.rb | 4 ++-- lib/protobuf/rpc/connectors/em_client.rb | 2 +- lib/protobuf/rpc/connectors/socket.rb | 18 +++++++++++++++--- lib/protobuf/rpc/servers/socket_server.rb | 2 +- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/lib/protobuf/rpc/buffer.rb b/lib/protobuf/rpc/buffer.rb index c48064bd..0cb2ab32 100644 --- a/lib/protobuf/rpc/buffer.rb +++ b/lib/protobuf/rpc/buffer.rb @@ -7,13 +7,13 @@ class Buffer MODES = [:read, :write] - def initialize mode=:read, data='' + def initialize(mode=:read, data='') @data = data.is_a?(Protobuf::Message) ? data.serialize_to_string : data.to_s @flush = false self.mode = mode end - def mode= mode + def mode=(mode) if MODES.include? mode @mode = mode else @@ -21,7 +21,7 @@ def mode= mode end end - def write force_mode=true + def write(force_mode=true) if force_mode and reading? mode = :write elsif not force_mode and reading? @@ -32,7 +32,7 @@ def write force_mode=true '%d-%s' % [@size, @data] end - def << data + def <<(data) @data << data if reading? get_data_size @@ -71,4 +71,4 @@ def check_for_flush end end -end \ No newline at end of file +end diff --git a/lib/protobuf/rpc/connectors/common.rb b/lib/protobuf/rpc/connectors/common.rb index 2f6531b3..eb41d85f 100644 --- a/lib/protobuf/rpc/connectors/common.rb +++ b/lib/protobuf/rpc/connectors/common.rb @@ -80,13 +80,13 @@ def parse_response # Setup the read buffer for data coming back def post_init log_debug '[client-cnxn] Post init, new read buffer created' - @buffer = Protobuf::Rpc::Buffer.new :read + @buffer = Protobuf::Rpc::Buffer.new(:read) rescue fail(:RPC_ERROR, 'Connection error: %s' % $!.message) unless failed? end # Sends the request to the server, invoked by the connection_completed event - def send_request + def _send_request request_wrapper = Protobuf::Socketrpc::Request.new request_wrapper.service_name = @options[:service].name request_wrapper.method_name = @options[:method].to_s diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index 1b9a5ea0..4159be7f 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -57,7 +57,7 @@ def on_complete(&complete_cb) # Called after the EM.connect def connection_completed log_debug '[client-cnxn] Established server connection, sending request' - send_request unless error? + _send_request unless error? rescue fail(:RPC_ERROR, 'Connection error: %s' % $!.message) unless failed? end diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index 13e1dba4..0b563d3b 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -10,21 +10,33 @@ def send_request post_init check_async connect_to_rpc_server + _send_request + read_response end private - def send_data(data) - end - def check_async raise "Cannot use Socket client in async mode" if async? end + def close_connection + @socket.close + end + def connect_to_rpc_server @socket = TCPSocket.new(options[:host], options[:port]) end + def read_response + @buffer << @socket.read + parse_response if @buffer.flushed? + end + + def send_data(data) + @socket.write(data) + end + end end end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index f74a32cb..beff4b65 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -46,7 +46,7 @@ def initialize(sock) @stats.client = Socket.unpack_sockaddr_in(sock.getpeername) @buffer << @socket.read - handle_client + handle_client if @buffer.flushed? end end From adbdf7a37690933017146dc1826ae4ac4c4f5f52 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 3 Dec 2011 18:42:38 -0700 Subject: [PATCH 12/50] allow connector type to be set in rpc_server --- bin/rpc_server | 4 ++++ lib/protobuf.rb | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/bin/rpc_server b/bin/rpc_server index 6b0c6a5e..96cff265 100755 --- a/bin/rpc_server +++ b/bin/rpc_server @@ -42,6 +42,10 @@ parser = OptionParser.new do |opts| server.level = v.to_i end + opts.on("-cs", "--client_socket", "Socket Mode for client connections (No EventMachine)") do |v| + Protobuf::ConnectorType = "Socket" + end + opts.on("-s", "--socket", "Socket Server Mode (No EventMachine)") do |v| Protobuf::ServerType = "SocketServer" server.runner = Protobuf::Rpc::SocketRunner diff --git a/lib/protobuf.rb b/lib/protobuf.rb index a265141f..7dcec27e 100644 --- a/lib/protobuf.rb +++ b/lib/protobuf.rb @@ -9,7 +9,6 @@ module Protobuf require 'protobuf/rpc/client' require 'protobuf/rpc/connectors/socket' else - Protobuf::ConnectorType = "EventMachine" require 'eventmachine' require 'protobuf/ext/eventmachine' require 'protobuf/rpc/client' From 833c21af32f125e5a339fd1934b588236d663d88 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 3 Dec 2011 18:45:04 -0700 Subject: [PATCH 13/50] be explicit on ConnectorType when loading --- lib/protobuf.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/protobuf.rb b/lib/protobuf.rb index 7dcec27e..a265141f 100644 --- a/lib/protobuf.rb +++ b/lib/protobuf.rb @@ -9,6 +9,7 @@ module Protobuf require 'protobuf/rpc/client' require 'protobuf/rpc/connectors/socket' else + Protobuf::ConnectorType = "EventMachine" require 'eventmachine' require 'protobuf/ext/eventmachine' require 'protobuf/rpc/client' From 21449260cfbf8582434333a7f1bb2b61498cdbca Mon Sep 17 00:00:00 2001 From: = Date: Sat, 3 Dec 2011 20:38:48 -0700 Subject: [PATCH 14/50] adding specs for connector abstraction --- lib/protobuf/rpc/servers/evented_runner.rb | 2 +- lib/protobuf/rpc/servers/evented_server.rb | 2 +- spec/helper/server.rb | 27 +++++++++++++++---- spec/unit/rpc/client_spec.rb | 8 +++--- ...nt_spec.rb => eventmachine_client_spec.rb} | 0 spec/unit/rpc/connectors/socket_spec.rb | 0 6 files changed, 28 insertions(+), 11 deletions(-) rename spec/unit/rpc/connectors/{eventmachine/client_spec.rb => eventmachine_client_spec.rb} (100%) create mode 100644 spec/unit/rpc/connectors/socket_spec.rb diff --git a/lib/protobuf/rpc/servers/evented_runner.rb b/lib/protobuf/rpc/servers/evented_runner.rb index f637544b..3add6e89 100644 --- a/lib/protobuf/rpc/servers/evented_runner.rb +++ b/lib/protobuf/rpc/servers/evented_runner.rb @@ -23,7 +23,7 @@ def run(server) # Startup and run the rpc server EM.schedule do - EventMachine.start_server(server.host, server.port, Protobuf::Rpc::Server) && \ + EventMachine.start_server(server.host, server.port, Protobuf::Rpc::EventedServer) && \ Protobuf::Logger.info('RPC Server listening at %s:%d in %s' % [server.host, server.port, server.env]) end diff --git a/lib/protobuf/rpc/servers/evented_server.rb b/lib/protobuf/rpc/servers/evented_server.rb index dcc5509a..cd9fc213 100644 --- a/lib/protobuf/rpc/servers/evented_server.rb +++ b/lib/protobuf/rpc/servers/evented_server.rb @@ -18,7 +18,7 @@ def post_init end # Receive a chunk of data, potentially flushed to handle_client - def receive_data data + def receive_data(data) log_debug '[server] receive_data: %s' % data @buffer << data handle_client if @buffer.flushed? diff --git a/spec/helper/server.rb b/spec/helper/server.rb index 944bac84..b1b5d729 100644 --- a/spec/helper/server.rb +++ b/spec/helper/server.rb @@ -1,9 +1,10 @@ +require 'ostruct' require 'protobuf/rpc/server' require 'spec/proto/test_service_impl' module StubProtobufServerFactory - def self.build(delay) - new_server = Class.new(Protobuf::Rpc::EventedServer) do + def self.build(delay, server_class) + new_server = Class.new(server_class) do class << self def sleep_interval @sleep_interval @@ -26,11 +27,27 @@ def post_init end class StubServer - def initialize(delay = 0, port = 9191) - @server_handle = EventMachine::start_server("127.0.0.1", port, StubProtobufServerFactory.build(delay)) + def initialize(opts = {}) + @options = OpenStruct.new({ + :host => "127.0.0.1", + :port => 9191, + :delay => 0, + :server => Protobuf::Rpc::EventedServer + }.merge(opts)) + + if @options.server == Protobuf::Rpc::EventedServer + @server_handle = EventMachine::start_server(@options.host, @options.port, StubProtobufServerFactory.build(@options.delay, @options.server)) + else + Protobuf::Rpc::SocketRunner.run(@options) + end end def stop - EventMachine.stop_server(@server_handle) + case + when @options.server == Protobuf::Rpc::EventedServer then + EventMachine.stop_server(@server_handle) + else + Protobuf::Rpc::SocketRunner.stop + end end end diff --git a/spec/unit/rpc/client_spec.rb b/spec/unit/rpc/client_spec.rb index 7b97ee3e..dc724063 100644 --- a/spec/unit/rpc/client_spec.rb +++ b/spec/unit/rpc/client_spec.rb @@ -7,7 +7,7 @@ it "waits for response when running synchronously" do EventMachine.fiber_run do delay = 3 - server = StubServer.new(delay) + server = StubServer.new(:delay => delay) stop_servers = lambda { server.stop; EventMachine.stop } client = Spec::Proto::TestService.client(:async => false) @@ -32,7 +32,7 @@ it "doesn't wait for response when running async call inside fiber" do EventMachine.fiber_run do delay = 3 - server = StubServer.new(delay) + server = StubServer.new(:delay => delay) stop_servers = lambda { server.stop; EventMachine.stop } client = Spec::Proto::TestService.client(:async => true) @@ -49,7 +49,7 @@ subject = Proc.new do EventMachine.run do delay = 1 - server = StubServer.new(delay) + server = StubServer.new(:delay => delay) stop_servers = lambda { server.stop; EventMachine.stop } client = Spec::Proto::TestService.client(:async => false) @@ -65,7 +65,7 @@ subject = Proc.new do EventMachine.fiber_run do delay = 3 - server = StubServer.new(delay) + server = StubServer.new(:delay => delay) stop_servers = lambda { server.stop; EventMachine.stop } client = Spec::Proto::TestService.client(:async => false, :timeout => 1) diff --git a/spec/unit/rpc/connectors/eventmachine/client_spec.rb b/spec/unit/rpc/connectors/eventmachine_client_spec.rb similarity index 100% rename from spec/unit/rpc/connectors/eventmachine/client_spec.rb rename to spec/unit/rpc/connectors/eventmachine_client_spec.rb diff --git a/spec/unit/rpc/connectors/socket_spec.rb b/spec/unit/rpc/connectors/socket_spec.rb new file mode 100644 index 00000000..e69de29b From 05c3a94f9f6ffb01f93958ff9e18f270ebf83912 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 3 Dec 2011 22:45:42 -0700 Subject: [PATCH 15/50] move specs for servers and use the blockness of methods in handling errors --- lib/protobuf/compiler/visitors.rb | 17 ++- lib/protobuf/rpc/server.rb | 131 +++++++++--------- lib/protobuf/rpc/servers/socket_server.rb | 1 + lib/protobuf/rpc/service.rb | 73 +++++----- spec/helper/silent_constants.rb | 13 +- .../rpc/{ => servers}/evented_server_spec.rb | 0 .../rpc/{ => servers}/socket_server_spec.rb | 4 +- 7 files changed, 113 insertions(+), 126 deletions(-) rename spec/unit/rpc/{ => servers}/evented_server_spec.rb (100%) rename spec/unit/rpc/{ => servers}/socket_server_spec.rb (98%) diff --git a/lib/protobuf/compiler/visitors.rb b/lib/protobuf/compiler/visitors.rb index 4a6adae2..f4a1861b 100644 --- a/lib/protobuf/compiler/visitors.rb +++ b/lib/protobuf/compiler/visitors.rb @@ -110,15 +110,14 @@ def required_message_from_proto(proto_file) end def create_files(filename, out_dir, file_create) - begin - $: << File.expand_path(out_dir) - Class.new.class_eval(to_s) # check the message - $:.delete File.expand_path(out_dir) - rescue LoadError - puts "Error creating file #{filename}" - puts $!.message - exit 1 - end + $: << File.expand_path(out_dir) + Class.new.class_eval(to_s) # check the message + $:.delete File.expand_path(out_dir) + rescue LoadError + puts "Error creating file #{filename}" + puts $!.message + exit 1 + else file = File.basename(filename) message_module = Util.module_to_path(package.map{|p| p.to_s.capitalize}.join('::')) diff --git a/lib/protobuf/rpc/server.rb b/lib/protobuf/rpc/server.rb index 57e31e94..385df2c8 100644 --- a/lib/protobuf/rpc/server.rb +++ b/lib/protobuf/rpc/server.rb @@ -27,7 +27,6 @@ def handle_client # Call the service method log_debug '[server] Dispatching client request to service' invoke_rpc_method - rescue => error # Ensure we're handling any errors that try to slip out the back door log_error error.message @@ -35,6 +34,19 @@ def handle_client handle_error(error) send_response end + + # Client error handler. Receives an exception object and writes it into the @response + def handle_error(error) + log_debug '[server] handle_error: %s' % error.inspect + if error.is_a?(PbError) + error.to_response(@response) + elsif error.is_a?(ClientError) + PbError.new(error.message, error.code.to_s).to_response(@response) + else + message = error.is_a?(String) ? error : error.message + PbError.new(message, 'RPC_ERROR').to_response(@response) + end + end # Assuming all things check out, we can call the service method def invoke_rpc_method @@ -68,49 +80,53 @@ def invoke_rpc_method # Parse the incoming request object into our expected request object def parse_request_from_buffer - begin - log_debug '[server] parsing request from buffer: %s' % @buffer.data.inspect - @request.parse_from_string @buffer.data - rescue => error - exc = BadRequestData.new 'Unable to parse request: %s' % error.message - log_error exc.message - raise exc - end + log_debug '[server] parsing request from buffer: %s' % @buffer.data.inspect + @request.parse_from_string @buffer.data + rescue => error + exc = BadRequestData.new 'Unable to parse request: %s' % error.message + log_error exc.message + raise exc end - + # Read out the response from the service method, # setting it on the pb request, and serializing the # response to the protobuf response wrapper - def parse_response_from_service response - begin - expected = @klass.rpcs[@klass][@method].response_type - - # Cannibalize the response if it's a Hash - response = expected.new(response) if response.is_a?(Hash) - actual = response.class - - log_debug '[server] response (should/actual): %s/%s' % [expected.name, actual.name] - - # Determine if the service tried to change response types on us - if expected == actual - begin - # Response types match, so go ahead and serialize - log_debug '[server] serializing response: %s' % response.inspect - @response.response_proto = response.serialize_to_string - rescue - raise BadResponseProto, $!.message - end - else - # response types do not match, throw the appropriate error - raise BadResponseProto, 'Response proto changed from %s to %s' % [expected.name, actual.name] - end - rescue => error - log_error error.message - log_error error.backtrace.join("\n") - handle_error(error) + def parse_response_from_service(response) + expected = @klass.rpcs[@klass][@method].response_type + + # Cannibalize the response if it's a Hash + response = expected.new(response) if response.is_a?(Hash) + actual = response.class + log_debug '[server] response (should/actual): %s/%s' % [expected.name, actual.name] + + # Determine if the service tried to change response types on us + if expected == actual + serialize_response(response) + else + # response types do not match, throw the appropriate error + raise BadResponseProto, 'Response proto changed from %s to %s' % [expected.name, actual.name] end + rescue => error + log_error error.message + log_error error.backtrace.join("\n") + handle_error(error) end - + + # Parses and returns the service and method name from the request wrapper proto + def parse_service_info + @klass = Util.constantize(@request.service_name) + @method = Util.underscore(@request.method_name).to_sym + + unless @klass.instance_methods.include?(@method) + raise MethodNotFound, "Service method #{@request.method_name} is not defined by the service" + end + + @stats.service = @klass.name + @stats.method = @method + rescue NameError + raise ServiceNotFound, "Service class #{@request.service_name} is not found" + end + # Write the response wrapper to the client def send_response raise 'Response already sent to client' if @did_respond @@ -122,39 +138,16 @@ def send_response @stats.log_stats @did_respond = true end - - # Client error handler. Receives an exception object and writes it into the @response - def handle_error error - log_debug '[server] handle_error: %s' % error.inspect - if error.is_a? PbError - error.to_response @response - elsif error.is_a? ClientError - PbError.new(error.message, error.code.to_s).to_response @response - else - message = error.is_a?(String) ? error : error.message - PbError.new(message, 'RPC_ERROR').to_response @response - end - end - - # Parses and returns the service and method name from the request wrapper proto - def parse_service_info - @klass, @method = nil, nil - - begin - @klass = Util.constantize(@request.service_name) - rescue - raise ServiceNotFound, "Service class #{@request.service_name} is not found" - end - - @method = Util.underscore(@request.method_name).to_sym - unless @klass.instance_methods.include?(@method) - raise MethodNotFound, "Service method #{@request.method_name} is not defined by the service" - end - - @stats.service = @klass.name - @stats.method = @method + + def serialize_response(response) + log_debug '[server] serializing response: %s' % response.inspect + @response.response_proto = response.serialize_to_string + rescue + raise BadResponseProto, $!.message end end + end + end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index beff4b65..e561f333 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -7,6 +7,7 @@ class SocketServer include Protobuf::Logger::LogMethods class << self + def stop @running = false @server.close diff --git a/lib/protobuf/rpc/service.rb b/lib/protobuf/rpc/service.rb index fe23d972..d68b29f5 100644 --- a/lib/protobuf/rpc/service.rb +++ b/lib/protobuf/rpc/service.rb @@ -4,7 +4,6 @@ module Protobuf module Rpc - # Object to encapsulate the request/response types for a given service method # RpcMethod = Struct.new "RpcMethod", :service, :method, :request_type, :response_type @@ -34,22 +33,20 @@ class << self # or it isn't in the reserved method list (NON_RPC_METHODS), # We want to remap the method such that we can wrap it in before and after behavior, # most notably calling call_rpc against the method. See call_rpc for more info. - def method_added old + def method_added(old) new_method = :"rpc_#{old}" return if private_instance_methods.include?(new_method) or old =~ /^rpc_/ or NON_RPC_METHODS.include?(old.to_s) alias_method new_method, old private new_method - begin - define_method(old) do |pb_request| - call_rpc old.to_sym, pb_request - end - rescue ArgumentError => e - # Wrap a known issue where an instance method was defined in the class without - # it being ignored with NON_RPC_METHODS. - raise ArgumentError, "#{e.message} (Note: This could mean that you need to add the method #{old} to the NON_RPC_METHODS list)" + define_method(old) do |pb_request| + call_rpc old.to_sym, pb_request end + rescue ArgumentError => e + # Wrap a known issue where an instance method was defined in the class without + # it being ignored with NON_RPC_METHODS. + raise ArgumentError, "#{e.message} (Note: This could mean that you need to add the method #{old} to the NON_RPC_METHODS list)" end # Generated service classes should call this method on themselves to add rpc methods @@ -85,8 +82,8 @@ def client options={} # def configure config={} locations[self] ||= {} - locations[self][:host] = config[:host] if config.key? :host - locations[self][:port] = config[:port] if config.key? :port + locations[self][:host] = config[:host] if config.key?(:host) + locations[self][:port] = config[:port] if config.key?(:port) end # Shorthand call to configure, passing a string formatted as hostname:port @@ -141,7 +138,7 @@ def rpcs # Callback register for the server when a service # method calls rpc_failed. Called by Service#rpc_failed. - def on_rpc_failed &rpc_failure_cb + def on_rpc_failed(&rpc_failure_cb) @rpc_failure_cb = rpc_failure_cb end @@ -149,12 +146,9 @@ def on_rpc_failed &rpc_failure_cb # NOTE: This shortcuts the @async_responder paradigm. There is # not any way to get around this currently (and I'm not sure you should want to). # - def rpc_failed message="RPC Failed while executing service method #{@current_method}" - if @rpc_failure_cb.nil? - exc = RuntimeError.new 'Unable to invoke rpc_failed, no failure callback is setup.' - log_error exc.message - raise exc - end + def rpc_failed(message="RPC Failed while executing service method #{@current_method}") + error_message = 'Unable to invoke rpc_failed, no failure callback is setup.' + log_and_raise_error(error_message) if @rpc_failure_cb.nil? error = message.is_a?(String) ? RpcFailed.new(message) : message log_warn '[service] RPC Failed: %s' % error.message @rpc_failure_cb.call(error) @@ -164,7 +158,7 @@ def rpc_failed message="RPC Failed while executing service method #{@current_met # when it is appropriate to generate a response to the client. # Used in conjunciton with Service#send_response. # - def on_send_response &responder + def on_send_response(&responder) @responder = responder end @@ -175,15 +169,17 @@ def on_send_response &responder # will timeout since no data will be sent. # def send_response - if @responder.nil? - exc = RuntimeError.new "Unable to send response, responder is nil. It appears you aren't inside of an RPC request/response cycle." - log_error exc.message - raise exc - end - @responder.call @response + error_message = "Unable to send response, responder is nil. It appears you aren't inside of an RPC request/response cycle." + log_and_raise_error(error_message) if @responder.nil? + @responder.call(@response) end private + + def log_and_raise_error(error_message) + logg_error(error_message) + raise error_message + end # Call the rpc method that was previously privatized. # call_rpc allows us to wrap the normal method call with @@ -202,24 +198,22 @@ def send_response # by calling self.send_response without any arguments. The rpc # server is setup to handle synchronous and asynchronous responses. # - def call_rpc method, pb_request + def call_rpc(method, pb_request) @current_method = method # Allows the service to set whether or not # it would like to asynchronously respond to the connected client(s) @async_responder = false - begin - # Setup the request - @request = rpcs[method].request_type.new - @request.parse_from_string pb_request.request_proto - rescue - exc = BadRequestProto.new 'Unable to parse request: %s' % $!.message - log_error exc.message - log_error $!.backtrace.join("\n") - raise exc - end - + # Setup the request + @request = rpcs[method].request_type.new + @request.parse_from_string(pb_request.request_proto) + rescue + exc = BadRequestProto.new 'Unable to parse request: %s' % $!.message + log_error exc.message + log_error $!.backtrace.join("\n") + raise exc + else # when no Exception was thrown # Setup the response @response = rpcs[method].response_type.new @@ -241,4 +235,5 @@ def call_rpc method, pb_request end end -end \ No newline at end of file + +end diff --git a/spec/helper/silent_constants.rb b/spec/helper/silent_constants.rb index c959f6ed..2b6b93a3 100644 --- a/spec/helper/silent_constants.rb +++ b/spec/helper/silent_constants.rb @@ -29,13 +29,12 @@ def with_constants(constants, &block) Kernel::silence_warnings { source_object.const_set(const_name, val) } end - begin - block.call - ensure - constants.each do |constant, val| - source_object, const_name = parse(constant) - Kernel::silence_warnings { source_object.const_set(const_name, saved_constants[constant]) } - end + block.call + ensure + constants.each do |constant, val| + source_object, const_name = parse(constant) + Kernel::silence_warnings { source_object.const_set(const_name, saved_constants[constant]) } end end + end diff --git a/spec/unit/rpc/evented_server_spec.rb b/spec/unit/rpc/servers/evented_server_spec.rb similarity index 100% rename from spec/unit/rpc/evented_server_spec.rb rename to spec/unit/rpc/servers/evented_server_spec.rb diff --git a/spec/unit/rpc/socket_server_spec.rb b/spec/unit/rpc/servers/socket_server_spec.rb similarity index 98% rename from spec/unit/rpc/socket_server_spec.rb rename to spec/unit/rpc/servers/socket_server_spec.rb index c8da9be1..c764f10b 100644 --- a/spec/unit/rpc/socket_server_spec.rb +++ b/spec/unit/rpc/servers/socket_server_spec.rb @@ -5,14 +5,14 @@ context 'when sending response objects' do it 'should be able to send a hash object as a response' do server = Protobuf::Rpc::SocketServer.new - + # Setup the right mocks server.instance_variable_set(:@klass, Spec::Proto::TestService) response_wrapper = mock('response') response_wrapper.stub(:response_proto=) server.instance_variable_set(:@response, response_wrapper) Spec::Proto::TestService.stub_chain(:rpcs, :[], :[], :response_type).and_return(Spec::Proto::ResourceFindRequest) - + # Setup expectations hash_response = {:name => 'Test Name', :active => false} expected = Spec::Proto::ResourceFindRequest.new(hash_response) From d75c0c775291d6559abe5c63a6f46472815671fb Mon Sep 17 00:00:00 2001 From: = Date: Sun, 4 Dec 2011 00:28:08 -0700 Subject: [PATCH 16/50] make sure EM is running for client in SocketServer scenario --- lib/protobuf.rb | 17 +++--- lib/protobuf/rpc/connectors/eventmachine.rb | 58 ++++++++++++--------- lib/protobuf/rpc/servers/socket_runner.rb | 3 +- lib/protobuf/rpc/servers/socket_server.rb | 12 ++--- spec/unit/rpc/servers/socket_server_spec.rb | 33 +++++++++++- 5 files changed, 80 insertions(+), 43 deletions(-) diff --git a/lib/protobuf.rb b/lib/protobuf.rb index a265141f..8da1b63c 100644 --- a/lib/protobuf.rb +++ b/lib/protobuf.rb @@ -5,24 +5,21 @@ module Protobuf end # When setting up a client -if defined?(Protobuf::ConnectorType) && Protobuf::ConnectorType == "Socket" - require 'protobuf/rpc/client' - require 'protobuf/rpc/connectors/socket' -else +unless defined?(Protobuf::ConnectorType) && Protobuf::ConnectorType == "Socket" Protobuf::ConnectorType = "EventMachine" require 'eventmachine' require 'protobuf/ext/eventmachine' - require 'protobuf/rpc/client' require 'protobuf/rpc/connectors/eventmachine' end # For running the rpc_server -if defined?(Protobuf::ServerType) && Protobuf::ServerType == "SocketServer" - require 'protobuf/rpc/service' - require 'protobuf/rpc/servers/socket_server' -else +unless defined?(Protobuf::ServerType) && Protobuf::ServerType == "SocketServer" require 'eventmachine' require 'protobuf/ext/eventmachine' - require 'protobuf/rpc/service' require 'protobuf/rpc/servers/evented_server' end + +require 'protobuf/rpc/client' +require 'protobuf/rpc/connectors/socket' +require 'protobuf/rpc/service' +require 'protobuf/rpc/servers/socket_server' diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index b5af8b72..6467299f 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -7,21 +7,21 @@ module Connectors class EventMachine < Base def send_request - Thread.new { EM.run } unless EM.reactor_running? + return ensure_em_running do + f = Fiber.current + + EM.schedule do + log_debug '[client] Scheduling EventMachine client request to be created on next tick' + cnxn = EMClient.connect(options, &ensure_cb) + cnxn.on_success(&success_cb) if success_cb + cnxn.on_failure(&ensure_cb) + cnxn.on_complete { resume_fiber(f) } unless async? + log_debug '[client] Connection scheduled' + end - f = Fiber.current - - EM.schedule do - log_debug '[client] Scheduling EventMachine client request to be created on next tick' - cnxn = EMClient.connect(options, &ensure_cb) - cnxn.on_success(&success_cb) if success_cb - cnxn.on_failure(&ensure_cb) - cnxn.on_complete { resume_fiber(f) } unless async? - log_debug '[client] Connection scheduled' + set_timeout_and_validate_fiber unless async? + true if async? end - - return set_timeout_and_validate_fiber unless async? - return true end # Returns a proc that ensures any errors will be returned to the client @@ -46,18 +46,14 @@ def ensure_cb private - def set_timeout_and_validate_fiber - @timeout_timer = EM::add_timer(@options[:timeout]) do - message = 'Client timeout of %d seconds expired' % @options[:timeout] - err = ClientError.new(Protobuf::Socketrpc::ErrorReason::RPC_ERROR, message) - ensure_cb.call(err) - end + def ensure_em_running(&blk) + return yield if EM.reactor_running? - Fiber.yield - rescue FiberError - message = "Synchronous calls must be in 'EM.fiber_run' block" - err = ClientError.new(Protobuf::Socketrpc::ErrorReason::RPC_ERROR, message) - ensure_cb.call(err) + if async? + @em_thread = Thread.new { EM.run(blk) } unless EM.reactor_running? + else + @em_thread = Thread.new { EM.fiber_run(blk) } unless EM.reactor_running? + end end def resume_fiber(fib) @@ -73,6 +69,20 @@ def resume_fiber(fib) ensure_cb.call(err) end + def set_timeout_and_validate_fiber + @timeout_timer = EM::add_timer(@options[:timeout]) do + message = 'Client timeout of %d seconds expired' % @options[:timeout] + err = ClientError.new(Protobuf::Socketrpc::ErrorReason::RPC_ERROR, message) + ensure_cb.call(err) + end + + Fiber.yield + rescue FiberError + message = "Synchronous calls must be in 'EM.fiber_run' block" + err = ClientError.new(Protobuf::Socketrpc::ErrorReason::RPC_ERROR, message) + ensure_cb.call(err) + end + end end end diff --git a/lib/protobuf/rpc/servers/socket_runner.rb b/lib/protobuf/rpc/servers/socket_runner.rb index 4e02cc01..70f629c7 100644 --- a/lib/protobuf/rpc/servers/socket_runner.rb +++ b/lib/protobuf/rpc/servers/socket_runner.rb @@ -3,7 +3,6 @@ module Rpc class SocketRunner class << self - def stop Protobuf::Rpc::SocketServer.stop Protobuf::Logger.info 'Shutdown complete' @@ -14,8 +13,8 @@ def run(server) Protobuf::Logger.info "SocketServer Running" Protobuf::Rpc::SocketServer.run(server.host, server.port) if !Protobuf::Rpc::SocketServer.running? end - end + end end end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index e561f333..fdc6ae16 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -13,10 +13,11 @@ def stop @server.close end - def run(host = "127.0.0.1", port = 9399) + # TODO make listen/backlog part of config + def run(host = "127.0.0.1", port = 9399, backlog = 100) @running = true @server = TCPServer.new(host, port) - @server.listen(100) + @server.listen(backlog) loop do Thread.new(@server.accept) do |sock| @@ -27,14 +28,13 @@ def run(host = "127.0.0.1", port = 9399) rescue Errno::EBADF # Closing the server causes the loop to raise an exception here - if running? - raise - end + raise if running? end def running? - defined?(@running) && @running + @running end + end def initialize(sock) diff --git a/spec/unit/rpc/servers/socket_server_spec.rb b/spec/unit/rpc/servers/socket_server_spec.rb index c764f10b..74540f64 100644 --- a/spec/unit/rpc/servers/socket_server_spec.rb +++ b/spec/unit/rpc/servers/socket_server_spec.rb @@ -1,7 +1,38 @@ require 'spec_helper' require 'spec/proto/test_service_impl' +require 'protobuf/rpc/servers/socket_runner' + +describe Protobuf::Rpc::SocketServer do + before(:all) do + server = OpenStruct.new(:server => "127.0.0.1", :port => 9399) + Thread.new { Protobuf::Rpc::SocketRunner.run(server) } + Thread.pass # Pass control to the SocketRunner thread to let it startup + end + + after(:all) do + Protobuf::Rpc::SocketRunner.stop + end + + it "signals the SocketServer is running" do + Protobuf::Rpc::SocketServer.running?.should be_true + end + + it "calls the service in the client request" do + client = Spec::Proto::TestService.client(:async => false) + + client.find(:name => "Test Name", :active => true) do |c| + c.on_success do |succ| + succ.name.should eq("Test Name") + succ.status.should eq(Spec::Proto::StatusType::Enabled) + end + + c.on_failure do |err| + raise err.inspect + end + end + end + -describe Protobuf::Rpc::Server do context 'when sending response objects' do it 'should be able to send a hash object as a response' do server = Protobuf::Rpc::SocketServer.new From 5baf810734cf401643ba28438d76867620370d5f Mon Sep 17 00:00:00 2001 From: = Date: Sun, 4 Dec 2011 00:42:17 -0700 Subject: [PATCH 17/50] do not need to check reactor if it is already running --- lib/protobuf/rpc/connectors/eventmachine.rb | 7 +++---- spec/unit/rpc/servers/socket_server_spec.rb | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index 6467299f..66eda6c1 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -19,8 +19,7 @@ def send_request log_debug '[client] Connection scheduled' end - set_timeout_and_validate_fiber unless async? - true if async? + async? ? true : set_timeout_and_validate_fiber end end @@ -50,9 +49,9 @@ def ensure_em_running(&blk) return yield if EM.reactor_running? if async? - @em_thread = Thread.new { EM.run(blk) } unless EM.reactor_running? + @em_thread = Thread.new { EM.run(blk) } else - @em_thread = Thread.new { EM.fiber_run(blk) } unless EM.reactor_running? + @em_thread = Thread.new { EM.fiber_run(blk) } end end diff --git a/spec/unit/rpc/servers/socket_server_spec.rb b/spec/unit/rpc/servers/socket_server_spec.rb index 74540f64..0d4b4f5b 100644 --- a/spec/unit/rpc/servers/socket_server_spec.rb +++ b/spec/unit/rpc/servers/socket_server_spec.rb @@ -32,7 +32,6 @@ end end - context 'when sending response objects' do it 'should be able to send a hash object as a response' do server = Protobuf::Rpc::SocketServer.new From 2a80885e8d1b39d9d0056de19bb183593305ff76 Mon Sep 17 00:00:00 2001 From: = Date: Sun, 4 Dec 2011 19:47:42 -0700 Subject: [PATCH 18/50] allow return of response without callbacks, use close_read/close_write for socket connector/servers --- lib/protobuf/common/logger.rb | 6 +-- lib/protobuf/rpc/buffer.rb | 2 +- lib/protobuf/rpc/client.rb | 4 +- lib/protobuf/rpc/connectors/common.rb | 55 +++++++++++++++------ lib/protobuf/rpc/connectors/em_client.rb | 5 -- lib/protobuf/rpc/connectors/eventmachine.rb | 6 +-- lib/protobuf/rpc/connectors/socket.rb | 2 + lib/protobuf/rpc/server.rb | 4 +- lib/protobuf/rpc/servers/evented_server.rb | 2 +- lib/protobuf/rpc/servers/socket_server.rb | 13 +++-- spec/proto/test_service.rb | 1 - spec/proto/test_service_impl.rb | 7 +-- spec/spec_helper.rb | 6 +++ spec/unit/rpc/servers/socket_server_spec.rb | 36 +++++++++----- 14 files changed, 96 insertions(+), 53 deletions(-) diff --git a/lib/protobuf/common/logger.rb b/lib/protobuf/common/logger.rb index 21ebbc59..54e3f08e 100644 --- a/lib/protobuf/common/logger.rb +++ b/lib/protobuf/common/logger.rb @@ -5,15 +5,11 @@ class << self attr_accessor :file, :level # One-line file/level configuration - def configure options + def configure(options) self.file = options[:file] if options[:file] self.level = options[:level] if options[:level] end - def configured? - ! instance.nil? - end - # Use to reset the instance def reset_device! self.file = self.level = @__instance = nil diff --git a/lib/protobuf/rpc/buffer.rb b/lib/protobuf/rpc/buffer.rb index 0cb2ab32..de494ebe 100644 --- a/lib/protobuf/rpc/buffer.rb +++ b/lib/protobuf/rpc/buffer.rb @@ -14,7 +14,7 @@ def initialize(mode=:read, data='') end def mode=(mode) - if MODES.include? mode + if MODES.include?(mode) @mode = mode else @mode = :read diff --git a/lib/protobuf/rpc/client.rb b/lib/protobuf/rpc/client.rb index 1a544ee8..e2cc56c6 100644 --- a/lib/protobuf/rpc/client.rb +++ b/lib/protobuf/rpc/client.rb @@ -25,7 +25,7 @@ class Client # :request => request # }) # - def initialize opts={} + def initialize(opts={}) raise "Invalid client configuration. Service must be defined." if opts[:service].nil? @connector = Connector.connector_for_client.new(opts) log_debug '[client] Initialized with options: %s' % opts.inspect @@ -71,7 +71,7 @@ def method_missing(method, *params) service = options[:service] unless service.rpcs[service].keys.include?(method) log_error '[client] %s#%s not rpc method, passing to super' % [service.name, method.to_s] - super method, *params + super(method, *params) else log_debug '[client] %s#%s' % [service.name, method.to_s] rpc = service.rpcs[service][method.to_sym] diff --git a/lib/protobuf/rpc/connectors/common.rb b/lib/protobuf/rpc/connectors/common.rb index eb41d85f..4db2de7b 100644 --- a/lib/protobuf/rpc/connectors/common.rb +++ b/lib/protobuf/rpc/connectors/common.rb @@ -2,6 +2,8 @@ module Protobuf module Rpc module Connectors module Common + ClientError = Struct.new("ClientError", :code, :message) + # For state tracking STATES = { :pending => 0, @@ -9,7 +11,13 @@ module Common :failed => 2, :completed => 3 } - + + def any_callbacks? + return [@complete_cb, @failure_cb, @success_cb].inject(false) do |reduction, cb| + reduction = (reduction || !cb.nil?) + end + end + def complete @state = STATES[:completed] @stats.end @@ -23,6 +31,15 @@ def complete raise end + def data_callback(data) + @used_data_callback = true + @data = data + end + + def error + @error || ClientError.new + end + def fail(code, message) @state = STATES[:failed] error.code = code.is_a?(Symbol) ? Protobuf::Socketrpc::ErrorReason.values[code] : code @@ -47,38 +64,42 @@ def initialize_stats def parse_response # Close up the connection as we no longer need it close_connection - + log_debug '[client-cnxn] Parsing response from server (connection closed)' @stats.response_size = @buffer.size - + # Parse out the raw response response_wrapper = Protobuf::Socketrpc::Response.new response_wrapper.parse_from_string(@buffer.data) - + # Determine success or failure based on parsed data - if response_wrapper.has_field? :error_reason + if response_wrapper.has_field?(:error_reason) log_debug '[client-cnxn] Error response parsed' - + # fail the call if we already know the client is failed # (don't try to parse out the response payload) - fail response_wrapper.error_reason, response_wrapper.error + fail(response_wrapper.error_reason, response_wrapper.error) else log_debug '[client-cnxn] Successful response parsed' - + # Ensure client_response is an instance response_type = @options[:response_type].new parsed = response_type.parse_from_string(response_wrapper.response_proto.to_s) - + if parsed.nil? and not response_wrapper.has_field?(:error_reason) - fail :BAD_RESPONSE_PROTO, 'Unable to parse response from server' + fail(:BAD_RESPONSE_PROTO, 'Unable to parse response from server') else + verify_callbacks succeed(parsed) + return @data if @used_data_callback end end end # Setup the read buffer for data coming back def post_init + # Setup an object for reponses without callbacks + @data = nil log_debug '[client-cnxn] Post init, new read buffer created' @buffer = Protobuf::Rpc::Buffer.new(:read) rescue @@ -90,7 +111,7 @@ def _send_request request_wrapper = Protobuf::Socketrpc::Request.new request_wrapper.service_name = @options[:service].name request_wrapper.method_name = @options[:method].to_s - + if @options[:request].class == @options[:request_type] request_wrapper.request_proto = @options[:request].serialize_to_string else @@ -98,13 +119,13 @@ def _send_request actual = @options[:request].class.name fail :INVALID_REQUEST_PROTO, 'Expected request type to be type of %s, got %s instead' % [expected, actual] end - + log_debug '[client-cnxn] Sending Request: %s' % request_wrapper.inspect request_buffer = Protobuf::Rpc::Buffer.new(:write, request_wrapper) - send_data(request_buffer.write) @stats.request_size = request_buffer.size + send_data(request_buffer.write) end - + def succeed(response) @state = STATES[:succeeded] log_debug '[client-cnxn] Server succeeded request (invoking on_success)' @@ -117,6 +138,12 @@ def succeed(response) complete end + def verify_callbacks + if !any_callbacks? + @success_cb = @failure_cb = self.method(:data_callback) + end + end + def verify_options # Verify the options that are necessary and merge them in [:service, :method, :host, :port].each do |opt| diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index 4159be7f..296b6e4c 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -2,7 +2,6 @@ module Protobuf module Rpc module Connectors - ClientError = Struct.new("ClientError", :code, :message) class EMClient < EM::Connection include Protobuf::Logger::LogMethods @@ -35,10 +34,6 @@ def initialize(options={}, &failure_cb) fail(:RPC_ERROR, 'Failed to initialize connection: %s' % $!.message) unless failed? end - def error - @error || ClientError.new - end - # Success callback registration def on_success(&success_cb) @success_cb = success_cb diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index 66eda6c1..de3d1c17 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -64,21 +64,21 @@ def resume_fiber(fib) log_error ex.backtrace.join("\n") message = 'Synchronous client failed: %s' % ex.message - err = ClientError.new(Protobuf::Socketrpc::ErrorReason::RPC_ERROR, message) + err = Protobuf::Rpc::Connectors::Common::ClientError.new(Protobuf::Socketrpc::ErrorReason::RPC_ERROR, message) ensure_cb.call(err) end def set_timeout_and_validate_fiber @timeout_timer = EM::add_timer(@options[:timeout]) do message = 'Client timeout of %d seconds expired' % @options[:timeout] - err = ClientError.new(Protobuf::Socketrpc::ErrorReason::RPC_ERROR, message) + err = Protobuf::Rpc::Connectors::Common::ClientError.new(Protobuf::Socketrpc::ErrorReason::RPC_ERROR, message) ensure_cb.call(err) end Fiber.yield rescue FiberError message = "Synchronous calls must be in 'EM.fiber_run' block" - err = ClientError.new(Protobuf::Socketrpc::ErrorReason::RPC_ERROR, message) + err = Protobuf::Rpc::Connectors::Common::ClientError.new(Protobuf::Socketrpc::ErrorReason::RPC_ERROR, message) ensure_cb.call(err) end diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index 0b563d3b..6131d485 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -7,6 +7,7 @@ class Socket < Base include Protobuf::Rpc::Connectors::Common def send_request + initialize_stats post_init check_async connect_to_rpc_server @@ -35,6 +36,7 @@ def read_response def send_data(data) @socket.write(data) + @socket.close_write end end diff --git a/lib/protobuf/rpc/server.rb b/lib/protobuf/rpc/server.rb index 385df2c8..8ae13ee4 100644 --- a/lib/protobuf/rpc/server.rb +++ b/lib/protobuf/rpc/server.rb @@ -40,7 +40,7 @@ def handle_error(error) log_debug '[server] handle_error: %s' % error.inspect if error.is_a?(PbError) error.to_response(@response) - elsif error.is_a?(ClientError) + elsif error.is_a?(Protobuf::Rpc::Connectors::Common::ClientError) PbError.new(error.message, error.code.to_s).to_response(@response) else message = error.is_a?(String) ? error : error.message @@ -75,7 +75,7 @@ def invoke_rpc_method # Call the service method log_debug '[server] Invoking %s#%s with request %s' % [@klass.name, @method, @request.inspect] - @service.__send__ @method, @request + @service.__send__(@method, @request) end # Parse the incoming request object into our expected request object diff --git a/lib/protobuf/rpc/servers/evented_server.rb b/lib/protobuf/rpc/servers/evented_server.rb index cd9fc213..1ff16d7a 100644 --- a/lib/protobuf/rpc/servers/evented_server.rb +++ b/lib/protobuf/rpc/servers/evented_server.rb @@ -13,7 +13,7 @@ def post_init @stats = Protobuf::Rpc::Stat.new(:SERVER, true) @stats.client = Socket.unpack_sockaddr_in(get_peername) - @buffer = Protobuf::Rpc::Buffer.new :read + @buffer = Protobuf::Rpc::Buffer.new(:read) @did_respond = false end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index fdc6ae16..50248bc4 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -19,11 +19,9 @@ def run(host = "127.0.0.1", port = 9399, backlog = 100) @server = TCPServer.new(host, port) @server.listen(backlog) - loop do - Thread.new(@server.accept) do |sock| - service_worker = self.new(sock) - sock.close - end + while (sock = @server.accept) + self.new(sock) + sock.close end rescue Errno::EBADF @@ -50,6 +48,11 @@ def initialize(sock) handle_client if @buffer.flushed? end + def send_data(data) + @socket.write(data) + @socket.close_write + end + end end end diff --git a/spec/proto/test_service.rb b/spec/proto/test_service.rb index cda40342..cc146ef7 100644 --- a/spec/proto/test_service.rb +++ b/spec/proto/test_service.rb @@ -1,5 +1,4 @@ require 'protobuf/rpc/service' -require File.dirname(__FILE__) + '/test.pb' ## !! DO NOT EDIT THIS FILE !! ## diff --git a/spec/proto/test_service_impl.rb b/spec/proto/test_service_impl.rb index 8edbf162..0e5bc866 100644 --- a/spec/proto/test_service_impl.rb +++ b/spec/proto/test_service_impl.rb @@ -1,9 +1,10 @@ -require 'spec/proto/test_service' +require 'protobuf/rpc/service' +require File.dirname(__FILE__) + '/test.pb' +require File.dirname(__FILE__) + '/test_service' module Spec module Proto class TestService - located_at "localhost:9191" # request -> Spec::Proto::ResourceFindRequest # response -> Spec::Proto::Resource @@ -14,4 +15,4 @@ def find end end -end \ No newline at end of file +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 338a4967..fc9da686 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -9,6 +9,12 @@ require 'protobuf/rpc/client' require File.dirname(__FILE__) + '/helper/all' +# Including a way to turn on debug logger for spec runs +if ENV["DEBUG"] + debug_log = File.expand_path('../debug_specs.log', File.dirname(__FILE__) ) + Protobuf::Logger.configure(:file => debug_log, :level => ::Logger::DEBUG) +end + RSpec.configure do |c| c.include(SilentConstants) c.include(Sander6::CustomMatchers) diff --git a/spec/unit/rpc/servers/socket_server_spec.rb b/spec/unit/rpc/servers/socket_server_spec.rb index 0d4b4f5b..c577d8ab 100644 --- a/spec/unit/rpc/servers/socket_server_spec.rb +++ b/spec/unit/rpc/servers/socket_server_spec.rb @@ -6,30 +6,44 @@ before(:all) do server = OpenStruct.new(:server => "127.0.0.1", :port => 9399) Thread.new { Protobuf::Rpc::SocketRunner.run(server) } - Thread.pass # Pass control to the SocketRunner thread to let it startup + sleep 0.5 # TODO figure out how to do this without sleep (Thread.pass didn't work either...hmmmm) end after(:all) do + sleep 5 Protobuf::Rpc::SocketRunner.stop end - it "signals the SocketServer is running" do + it "provides a stop method" do + described_class.respond_to?(:stop).should be_true + end + + it "provides a Runner class" do + runner_class = described_class.to_s.gsub(/Server/, "Runner") + expect { Protobuf::Util.constantize(runner_class) }.to_not raise_error + end + + it "signals the Server is running" do Protobuf::Rpc::SocketServer.running?.should be_true end - it "calls the service in the client request" do - client = Spec::Proto::TestService.client(:async => false) + context "Eventmachine client" do - client.find(:name => "Test Name", :active => true) do |c| - c.on_success do |succ| - succ.name.should eq("Test Name") - succ.status.should eq(Spec::Proto::StatusType::Enabled) - end + it "calls the service in the client request" do + client = Spec::Proto::TestService.client(:async => false, :port => 9399, :host => "127.0.0.1") + + client.find(:name => "Test Name", :active => true) do |c| + c.on_success do |succ| + succ.name.should eq("Test Name") + succ.status.should eq(Spec::Proto::StatusType::Enabled) + end - c.on_failure do |err| - raise err.inspect + c.on_failure do |err| + raise err.inspect + end end end + end context 'when sending response objects' do From ed39fd789f0f325f381471e4d594dd5b99f420b4 Mon Sep 17 00:00:00 2001 From: = Date: Sun, 4 Dec 2011 20:22:49 -0700 Subject: [PATCH 19/50] make sure reactor is running before moving on --- lib/protobuf/rpc/connectors/eventmachine.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index de3d1c17..6996ec79 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -46,12 +46,15 @@ def ensure_cb private def ensure_em_running(&blk) - return yield if EM.reactor_running? - - if async? + case + when EM.reactor_running? then + blk.call + when async? then @em_thread = Thread.new { EM.run(blk) } + Thread.pass until EM.reactor_running? else @em_thread = Thread.new { EM.fiber_run(blk) } + Thread.pass until EM.reactor_running? end end From 50dc552ae04f427bf37cda557fdb67406beab773 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 6 Dec 2011 13:37:40 -0700 Subject: [PATCH 20/50] make sure reactor is running on stubserver --- spec/helper/server.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/spec/helper/server.rb b/spec/helper/server.rb index b1b5d729..be393053 100644 --- a/spec/helper/server.rb +++ b/spec/helper/server.rb @@ -36,12 +36,21 @@ def initialize(opts = {}) }.merge(opts)) if @options.server == Protobuf::Rpc::EventedServer - @server_handle = EventMachine::start_server(@options.host, @options.port, StubProtobufServerFactory.build(@options.delay, @options.server)) + start_em_server else Protobuf::Rpc::SocketRunner.run(@options) end end + def start_em_server + if !EM.reactor_running? + Thread.new { EM.run } + Thread.pass until EM.reactor_running? + end + + @server_handle = EventMachine::start_server(@options.host, @options.port, StubProtobufServerFactory.build(@options.delay, @options.server)) + end + def stop case when @options.server == Protobuf::Rpc::EventedServer then From 856fcca635280fc03955c7a1d4f25af59f1495fc Mon Sep 17 00:00:00 2001 From: = Date: Tue, 6 Dec 2011 14:40:14 -0700 Subject: [PATCH 21/50] remove states, start introduction of ruby socket for EM IO object --- lib/protobuf/rpc/connectors/common.rb | 14 +------- lib/protobuf/rpc/connectors/em_client.rb | 43 ++++++++--------------- lib/protobuf/rpc/servers/socket_server.rb | 1 - 3 files changed, 16 insertions(+), 42 deletions(-) diff --git a/lib/protobuf/rpc/connectors/common.rb b/lib/protobuf/rpc/connectors/common.rb index 4db2de7b..44293899 100644 --- a/lib/protobuf/rpc/connectors/common.rb +++ b/lib/protobuf/rpc/connectors/common.rb @@ -4,14 +4,6 @@ module Connectors module Common ClientError = Struct.new("ClientError", :code, :message) - # For state tracking - STATES = { - :pending => 0, - :succeeded => 1, - :failed => 2, - :completed => 3 - } - def any_callbacks? return [@complete_cb, @failure_cb, @success_cb].inject(false) do |reduction, cb| reduction = (reduction || !cb.nil?) @@ -19,12 +11,10 @@ def any_callbacks? end def complete - @state = STATES[:completed] @stats.end @stats.log_stats log_debug '[client-cnxn] Response proceessing complete' - @success_cb = @failure_cb = nil - @complete_cb.call(@state) unless @complete_cb.nil? + @complete_cb.call(self) unless @complete_cb.nil? rescue log_error '[client-cnxn] Complete callback error encountered: %s' % $!.message log_error '[client-cnxn] %s' % $!.backtrace.join("\n") @@ -41,7 +31,6 @@ def error end def fail(code, message) - @state = STATES[:failed] error.code = code.is_a?(Symbol) ? Protobuf::Socketrpc::ErrorReason.values[code] : code error.message = message log_debug '[client-cnxn] Server failed request (invoking on_failure): %s' % error.inspect @@ -127,7 +116,6 @@ def _send_request end def succeed(response) - @state = STATES[:succeeded] log_debug '[client-cnxn] Server succeeded request (invoking on_success)' @success_cb.call(response) unless @success_cb.nil? rescue diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index 296b6e4c..2c9ac99f 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -15,19 +15,30 @@ class << self def connect(options={}) options = DEFAULT_OPTIONS.merge(options) Protobuf::Logger.debug '[client-cnxn] Connecting to server: %s' % options.inspect - EM.connect(options[:host], options[:port], self, options) +# socket = TCPSocket.new(options[:host], options[:port]) +# EM.attach(socket, self, socket, options) + EM.connect(options[:host], options[:port], self, nil, options) end end + + # Called after the EM.connect + def connection_completed + log_debug '[client-cnxn] Established server connection, sending request' + _send_request unless error? +# @socket.close_write + rescue + fail(:RPC_ERROR, 'Connection error: %s' % $!.message) + end - def initialize(options={}, &failure_cb) + def initialize(socket, options={}, &failure_cb) + @socket = socket @failure_cb = failure_cb @options = DEFAULT_OPTIONS.merge(options) verify_options log_debug '[client-cnxn] Client Initialized: %s' % options.inspect @success_cb = nil - @state = STATES[:pending] initialize_stats rescue @@ -49,36 +60,12 @@ def on_complete(&complete_cb) @complete_cb = complete_cb end - # Called after the EM.connect - def connection_completed - log_debug '[client-cnxn] Established server connection, sending request' - _send_request unless error? - rescue - fail(:RPC_ERROR, 'Connection error: %s' % $!.message) unless failed? - end - def receive_data(data) log_debug '[client-cnxn] receive_data: %s' % data @buffer << data parse_response if @buffer.flushed? end - - def pending? - @state == STATES[:pending] - end - - def succeeded? - @state == STATES[:succeeded] - end - - def failed? - @state == STATES[:failed] - end - - def completed? - @state == STATES[:completed] - end - + end end end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index 50248bc4..f121ff63 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -50,7 +50,6 @@ def initialize(sock) def send_data(data) @socket.write(data) - @socket.close_write end end From 5f3821b43887ef2e89d8600a4906b454735ae861 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 6 Dec 2011 14:50:39 -0700 Subject: [PATCH 22/50] change PbError to duck typing --- lib/protobuf/rpc/server.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/protobuf/rpc/server.rb b/lib/protobuf/rpc/server.rb index 8ae13ee4..0981ac35 100644 --- a/lib/protobuf/rpc/server.rb +++ b/lib/protobuf/rpc/server.rb @@ -38,13 +38,12 @@ def handle_client # Client error handler. Receives an exception object and writes it into the @response def handle_error(error) log_debug '[server] handle_error: %s' % error.inspect - if error.is_a?(PbError) + if error.respond_to?(:to_response) error.to_response(@response) - elsif error.is_a?(Protobuf::Rpc::Connectors::Common::ClientError) - PbError.new(error.message, error.code.to_s).to_response(@response) else - message = error.is_a?(String) ? error : error.message - PbError.new(message, 'RPC_ERROR').to_response(@response) + message = error.respond_to?(:message) ? error.message : error.to_s + code = error.respond_to?(:code) ? error.code.to_s : "RPC_ERROR" + PbError.new(message, code).to_response(@response) end end From f2b8a486a0f5738d59c7c485d8c25344320cde14 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 6 Dec 2011 15:49:05 -0700 Subject: [PATCH 23/50] clean up callable in eventmachine --- lib/protobuf/rpc/connectors/eventmachine.rb | 12 +----------- spec/helper/server.rb | 6 +++--- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index 6996ec79..c4bb95a2 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -30,17 +30,7 @@ def send_request # don't want to swallow the black holes. # def ensure_cb - @ensure_cb ||= begin - cbk = nil - if @failure_cb - cbk = @failure_cb - else - cbk = proc do |error| - raise '%s: %s' % [error.code.name, error.message] - end - end - cbk - end + @ensure_cb ||= (@failure_cb || lambda { |error| raise '%s: %s' % [error.code.name, error.message] } ) end private diff --git a/spec/helper/server.rb b/spec/helper/server.rb index be393053..0d11ebbc 100644 --- a/spec/helper/server.rb +++ b/spec/helper/server.rb @@ -3,8 +3,8 @@ require 'spec/proto/test_service_impl' module StubProtobufServerFactory - def self.build(delay, server_class) - new_server = Class.new(server_class) do + def self.build(delay) + new_server = Class.new(Protobuf::Rpc::EventedServer) do class << self def sleep_interval @sleep_interval @@ -48,7 +48,7 @@ def start_em_server Thread.pass until EM.reactor_running? end - @server_handle = EventMachine::start_server(@options.host, @options.port, StubProtobufServerFactory.build(@options.delay, @options.server)) + @server_handle = EventMachine::start_server(@options.host, @options.port, StubProtobufServerFactory.build(@options.delay)) end def stop From bc7ef9abdafdb35192a96490f4b56c1ef575a93f Mon Sep 17 00:00:00 2001 From: = Date: Tue, 6 Dec 2011 16:25:12 -0700 Subject: [PATCH 24/50] using yield for block calling instead of call --- lib/protobuf/rpc/connectors/eventmachine.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index c4bb95a2..55f3b8b2 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -23,7 +23,7 @@ def send_request end end - # Returns a proc that ensures any errors will be returned to the client + # Returns a callable that ensures any errors will be returned to the client # # If a failure callback was set, just use that as a direct assignment # otherwise implement one here that simply throws an exception, since we @@ -38,7 +38,7 @@ def ensure_cb def ensure_em_running(&blk) case when EM.reactor_running? then - blk.call + yield when async? then @em_thread = Thread.new { EM.run(blk) } Thread.pass until EM.reactor_running? From 108a04cdf50ba8f048f2b05f634fb7659f8ea36c Mon Sep 17 00:00:00 2001 From: = Date: Tue, 6 Dec 2011 16:35:16 -0700 Subject: [PATCH 25/50] make sure failed? is removed --- lib/protobuf/rpc/connectors/common.rb | 2 +- lib/protobuf/rpc/connectors/em_client.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/protobuf/rpc/connectors/common.rb b/lib/protobuf/rpc/connectors/common.rb index 44293899..15cec7b7 100644 --- a/lib/protobuf/rpc/connectors/common.rb +++ b/lib/protobuf/rpc/connectors/common.rb @@ -92,7 +92,7 @@ def post_init log_debug '[client-cnxn] Post init, new read buffer created' @buffer = Protobuf::Rpc::Buffer.new(:read) rescue - fail(:RPC_ERROR, 'Connection error: %s' % $!.message) unless failed? + fail(:RPC_ERROR, 'Connection error: %s' % $!.message) end # Sends the request to the server, invoked by the connection_completed event diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index 2c9ac99f..6be79af9 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -42,7 +42,7 @@ def initialize(socket, options={}, &failure_cb) initialize_stats rescue - fail(:RPC_ERROR, 'Failed to initialize connection: %s' % $!.message) unless failed? + fail(:RPC_ERROR, 'Failed to initialize connection: %s' % $!.message) end # Success callback registration From 0755ae0e25a4e26b58fd43bc8635eca903ac5580 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 7 Dec 2011 00:19:17 -0700 Subject: [PATCH 26/50] add tests for common api methods and use attach for EM --- lib/protobuf/rpc/connectors/common.rb | 5 +++++ lib/protobuf/rpc/connectors/em_client.rb | 17 ++++------------- lib/protobuf/rpc/connectors/socket.rb | 5 +++++ spec/helper/server.rb | 6 +++++- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/lib/protobuf/rpc/connectors/common.rb b/lib/protobuf/rpc/connectors/common.rb index 15cec7b7..75a21ede 100644 --- a/lib/protobuf/rpc/connectors/common.rb +++ b/lib/protobuf/rpc/connectors/common.rb @@ -48,6 +48,9 @@ def initialize_stats @stats.server = [@options[:port], @options[:host]] @stats.service = @options[:service].name @stats.method = @options[:method] + self + rescue => ex + fail(:RPC_ERROR, "Invalid stats configuration. #{ex.message}") end def parse_response @@ -91,6 +94,8 @@ def post_init @data = nil log_debug '[client-cnxn] Post init, new read buffer created' @buffer = Protobuf::Rpc::Buffer.new(:read) + _send_request unless error? + log_debug '[client-cnxn] Post init, new read buffer created just sent' rescue fail(:RPC_ERROR, 'Connection error: %s' % $!.message) end diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index 6be79af9..274ad0e3 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -15,22 +15,13 @@ class << self def connect(options={}) options = DEFAULT_OPTIONS.merge(options) Protobuf::Logger.debug '[client-cnxn] Connecting to server: %s' % options.inspect -# socket = TCPSocket.new(options[:host], options[:port]) -# EM.attach(socket, self, socket, options) - EM.connect(options[:host], options[:port], self, nil, options) + # Using 'attach' to get access to a Ruby socket if needed + socket = TCPSocket.new(options[:host], options[:port]) + EM.attach(socket, self, socket, options) end end - - # Called after the EM.connect - def connection_completed - log_debug '[client-cnxn] Established server connection, sending request' - _send_request unless error? -# @socket.close_write - rescue - fail(:RPC_ERROR, 'Connection error: %s' % $!.message) - end - + def initialize(socket, options={}, &failure_cb) @socket = socket @failure_cb = failure_cb diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index 6131d485..9671ef9c 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -29,6 +29,11 @@ def connect_to_rpc_server @socket = TCPSocket.new(options[:host], options[:port]) end + # Method to determine error state, must be used with Connector api + def error? + @socket.closed? + end + def read_response @buffer << @socket.read parse_response if @buffer.flushed? diff --git a/spec/helper/server.rb b/spec/helper/server.rb index 0d11ebbc..01f8e8fa 100644 --- a/spec/helper/server.rb +++ b/spec/helper/server.rb @@ -1,4 +1,5 @@ require 'ostruct' +require 'protobuf/common/logger' require 'protobuf/rpc/server' require 'spec/proto/test_service_impl' @@ -27,10 +28,12 @@ def post_init end class StubServer + include Protobuf::Logger::LogMethods + def initialize(opts = {}) @options = OpenStruct.new({ :host => "127.0.0.1", - :port => 9191, + :port => 9939, :delay => 0, :server => Protobuf::Rpc::EventedServer }.merge(opts)) @@ -40,6 +43,7 @@ def initialize(opts = {}) else Protobuf::Rpc::SocketRunner.run(@options) end + log_debug "[stub-server] Server started #{@options.host}:#{@options.port}" end def start_em_server From 30fe7533602b6d4b91265d7e091c2dafcc9aa26d Mon Sep 17 00:00:00 2001 From: = Date: Wed, 7 Dec 2011 09:25:09 -0700 Subject: [PATCH 27/50] add common spec --- spec/unit/rpc/connectors/common_spec.rb | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 spec/unit/rpc/connectors/common_spec.rb diff --git a/spec/unit/rpc/connectors/common_spec.rb b/spec/unit/rpc/connectors/common_spec.rb new file mode 100644 index 00000000..dee90527 --- /dev/null +++ b/spec/unit/rpc/connectors/common_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' +require 'protobuf/rpc/service' + +describe Protobuf::Rpc::Connectors::Common do + let(:common_class) do + Class.new(Protobuf::Rpc::Connectors::Base) do + include Protobuf::Rpc::Connectors::Common + attr_accessor :options + end + end + + subject{ common_class.new({}) } + + # Check our definition of the "API" + specify{ subject.respond_to?(:any_callbacks?).should be_true } + specify{ subject.respond_to?(:data_callback).should be_true } + specify{ subject.respond_to?(:error).should be_true } + specify{ subject.respond_to?(:fail).should be_true } + specify{ subject.respond_to?(:complete).should be_true } + specify{ subject.respond_to?(:parse_response).should be_true } + specify{ subject.respond_to?(:_send_request).should be_true } + specify{ subject.respond_to?(:verify_options).should be_true } + specify{ subject.respond_to?(:verify_callbacks).should be_true } + +end From 0b472ecff3d5cf5fd63812977729fe3e3cd49279 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 7 Dec 2011 16:41:18 -0700 Subject: [PATCH 28/50] add specs for common --- lib/protobuf/rpc/connectors/socket.rb | 2 +- spec/unit/rpc/connectors/base_spec.rb | 4 + spec/unit/rpc/connectors/common_spec.rb | 135 +++++++++++++++++++++--- 3 files changed, 126 insertions(+), 15 deletions(-) diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index 9671ef9c..a0c1aa5c 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -7,9 +7,9 @@ class Socket < Base include Protobuf::Rpc::Connectors::Common def send_request + check_async initialize_stats post_init - check_async connect_to_rpc_server _send_request read_response diff --git a/spec/unit/rpc/connectors/base_spec.rb b/spec/unit/rpc/connectors/base_spec.rb index 1ef3e69d..87523d08 100644 --- a/spec/unit/rpc/connectors/base_spec.rb +++ b/spec/unit/rpc/connectors/base_spec.rb @@ -8,6 +8,10 @@ subject { Protobuf::Rpc::Connectors::Base.new(opts) } + it "raising an error when 'send_request' is not overridden" do + expect{ subject.send_request }.to raise_error(RuntimeError, /inherit a Connector/) + end + describe '.new' do it 'assigns passed options and initializes success/failure callbacks' do subject.options.should eq(Protobuf::Rpc::Connectors::DEFAULT_OPTIONS.merge(opts)) diff --git a/spec/unit/rpc/connectors/common_spec.rb b/spec/unit/rpc/connectors/common_spec.rb index dee90527..72d9c6d6 100644 --- a/spec/unit/rpc/connectors/common_spec.rb +++ b/spec/unit/rpc/connectors/common_spec.rb @@ -1,25 +1,132 @@ -require 'spec_helper' -require 'protobuf/rpc/service' +require 'spec_helper' +require 'protobuf/rpc/service' describe Protobuf::Rpc::Connectors::Common do let(:common_class) do Class.new(Protobuf::Rpc::Connectors::Base) do include Protobuf::Rpc::Connectors::Common attr_accessor :options + attr_accessor :stats end end - subject{ common_class.new({}) } - - # Check our definition of the "API" - specify{ subject.respond_to?(:any_callbacks?).should be_true } - specify{ subject.respond_to?(:data_callback).should be_true } - specify{ subject.respond_to?(:error).should be_true } - specify{ subject.respond_to?(:fail).should be_true } - specify{ subject.respond_to?(:complete).should be_true } - specify{ subject.respond_to?(:parse_response).should be_true } - specify{ subject.respond_to?(:_send_request).should be_true } - specify{ subject.respond_to?(:verify_options).should be_true } - specify{ subject.respond_to?(:verify_callbacks).should be_true } + subject{ @subject ||= common_class.new({}) } + + context "API" do + specify{ subject.respond_to?(:any_callbacks?).should be_true } + specify{ subject.respond_to?(:data_callback).should be_true } + specify{ subject.respond_to?(:error).should be_true } + specify{ subject.respond_to?(:fail).should be_true } + specify{ subject.respond_to?(:complete).should be_true } + specify{ subject.respond_to?(:parse_response).should be_true } + specify{ subject.respond_to?(:_send_request).should be_true } + specify{ subject.respond_to?(:verify_options).should be_true } + specify{ subject.respond_to?(:verify_callbacks).should be_true } + end + + context "#any_callbacks?" do + + [:@complete_cb, :@success_cb, :@failure_cb].each do |cb| + it "returns true if #{cb} is provided" do + subject.instance_variable_set(cb, "something") + subject.any_callbacks?.should be_true + end + end + + it "returns false when all callbacks are not provided" do + subject.instance_variable_set(:@complete_cb, nil) + subject.instance_variable_set(:@success_cb, nil) + subject.instance_variable_set(:@failure_cb, nil) + + subject.any_callbacks?.should be_false + end + + end + + context "#data_callback" do + it "changes state to use the data callback" do + subject.data_callback("data") + subject.instance_variable_get(:@used_data_callback).should be_true + end + + it "sets the data var when using the data_callback" do + subject.data_callback("data") + subject.instance_variable_get(:@data).should eq("data") + end + end + + context "#verify_callbacks" do + + it "sets @failure_cb to #data_callback when no callbacks are defined" do + subject.verify_callbacks + subject.instance_variable_get(:@failure_cb).should eq(subject.method(:data_callback)) + end + + it "sets @success_cb to #data_callback when no callbacks are defined" do + subject.verify_callbacks + subject.instance_variable_get(:@success_cb).should eq(subject.method(:data_callback)) + end + + it "doesn't set @failure_cb when already defined" do + set_cb = lambda{ true } + subject.instance_variable_set(:@failure_cb, set_cb) + subject.verify_callbacks + subject.instance_variable_get(:@failure_cb).should eq(set_cb) + subject.instance_variable_get(:@failure_cb).should_not eq(subject.method(:data_callback)) + end + + it "doesn't set @success_cb when already defined" do + set_cb = lambda{ true } + subject.instance_variable_set(:@success_cb, set_cb) + subject.verify_callbacks + subject.instance_variable_get(:@success_cb).should eq(set_cb) + subject.instance_variable_get(:@success_cb).should_not eq(subject.method(:data_callback)) + end + + end + + shared_examples "a ConnectorDisposition" do |meth, cb, *args| + + it "calls #complete before exit" do + stats = double("Object") + stats.stub(:end) { true } + stats.stub(:log_stats) { true } + subject.stats = stats + + subject.should_receive(:complete) + subject.method(meth).call(*args) + end + + it "calls the #{cb} callback when provided" do + stats = double("Object") + stats.stub(:end) { true } + stats.stub(:log_stats) { true } + subject.stats = stats + _cb = double("Object") + + subject.instance_variable_set("@#{cb}", _cb) + _cb.should_receive(:call).and_return(true) + subject.method(meth).call(*args) + end + + it "calls the complete callback when provided" do + stats = double("Object") + stats.stub(:end) { true } + stats.stub(:log_stats) { true } + subject.stats = stats + comp_cb = double("Object") + + subject.instance_variable_set(:@complete_cb, comp_cb) + comp_cb.should_receive(:call).and_return(true) + subject.method(meth).call(*args) + end + + end + + it_behaves_like("a ConnectorDisposition", :fail, "failure_cb", "code", "message") + it_behaves_like("a ConnectorDisposition", :fail, "complete_cb", "code", "message") + it_behaves_like("a ConnectorDisposition", :succeed, "complete_cb", "response") + it_behaves_like("a ConnectorDisposition", :succeed, "success_cb", "response") + it_behaves_like("a ConnectorDisposition", :complete, "complete_cb") end From 31396f5243cc380ae7c56c57718d62cb9a4a72e2 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 7 Dec 2011 17:58:22 -0700 Subject: [PATCH 29/50] example and counterexample for send_request --- spec/unit/rpc/connectors/base_spec.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/spec/unit/rpc/connectors/base_spec.rb b/spec/unit/rpc/connectors/base_spec.rb index 87523d08..fd2c8b07 100644 --- a/spec/unit/rpc/connectors/base_spec.rb +++ b/spec/unit/rpc/connectors/base_spec.rb @@ -8,8 +8,16 @@ subject { Protobuf::Rpc::Connectors::Base.new(opts) } - it "raising an error when 'send_request' is not overridden" do - expect{ subject.send_request }.to raise_error(RuntimeError, /inherit a Connector/) + describe "#send_request" do + it "raising an error when 'send_request' is not overridden" do + expect{ subject.send_request }.to raise_error(RuntimeError, /inherit a Connector/) + end + + it "does not raise error when 'send_request' is overridden" do + new_sub = Class.new(subject.class){ def send_request; end } + new_sub = new_sub.new(opts) + expect{ new_sub.send_request }.to_not raise_error + end end describe '.new' do From 3d10b93f888670b442d72cbe5fb14ac54be98f4f Mon Sep 17 00:00:00 2001 From: = Date: Wed, 7 Dec 2011 20:53:29 -0700 Subject: [PATCH 30/50] adding shared examples for connectors and specs in socket connector --- spec/unit/rpc/connectors/base_spec.rb | 7 +++-- spec/unit/rpc/connectors/socket_spec.rb | 35 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/spec/unit/rpc/connectors/base_spec.rb b/spec/unit/rpc/connectors/base_spec.rb index fd2c8b07..380fd69b 100644 --- a/spec/unit/rpc/connectors/base_spec.rb +++ b/spec/unit/rpc/connectors/base_spec.rb @@ -14,8 +14,7 @@ end it "does not raise error when 'send_request' is overridden" do - new_sub = Class.new(subject.class){ def send_request; end } - new_sub = new_sub.new(opts) + new_sub = Class.new(subject.class){ def send_request; end }.new(opts) expect{ new_sub.send_request }.to_not raise_error end end @@ -33,7 +32,7 @@ subject.success_cb.should be_nil cb = proc {|res| raise res } subject.success_cb = cb - subject.success_cb.should eq cb + subject.success_cb.should eq(cb) expect { subject.success_cb.call('an error from cb') }.to raise_error 'an error from cb' end end @@ -43,7 +42,7 @@ subject.failure_cb.should be_nil cb = proc {|res| raise res } subject.failure_cb = cb - subject.failure_cb.should eq cb + subject.failure_cb.should eq(cb) expect { subject.failure_cb.call('an error from cb') }.to raise_error 'an error from cb' end end diff --git a/spec/unit/rpc/connectors/socket_spec.rb b/spec/unit/rpc/connectors/socket_spec.rb index e69de29b..1c0cd2de 100644 --- a/spec/unit/rpc/connectors/socket_spec.rb +++ b/spec/unit/rpc/connectors/socket_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +shared_examples "a Protobuf Connector" do + + subject{ described_class.new({}) } + + context "API" do + # Check the API + specify{ subject.respond_to?(:send_request, true).should be_true } + specify{ subject.respond_to?(:post_init, true).should be_true } + specify{ subject.respond_to?(:close_connection, true).should be_true } + specify{ subject.respond_to?(:error?, true).should be_true } + end + +end + +describe Protobuf::Rpc::Connectors::Socket do + + specify{ described_class.include?(Protobuf::Rpc::Connectors::Common).should be_true } + + context "#check_async" do + it "raises an error when trying to execute asynchronously" do + conn = described_class.new(:async => true) + expect{ conn.__send__(:check_async) }.to raise_error + end + + it "allows execution when synchronous" do + conn = described_class.new(:async => false) + expect{ conn.__send__(:check_async) }.to_not raise_error + end + end + + it_behaves_like "a Protobuf Connector" + +end From 5c20dfa09f7d1c2859d23121e785929ccb458a9c Mon Sep 17 00:00:00 2001 From: = Date: Thu, 8 Dec 2011 14:41:29 -0700 Subject: [PATCH 31/50] make stub server block based for auto stopping --- lib/protobuf/rpc/client.rb | 39 +++++++++-- lib/protobuf/rpc/connectors/base.rb | 2 +- lib/protobuf/rpc/connectors/em_client.rb | 3 +- lib/protobuf/rpc/connectors/eventmachine.rb | 6 +- lib/protobuf/rpc/connectors/socket.rb | 3 +- lib/protobuf/rpc/service.rb | 8 +-- spec/helper/server.rb | 24 +++++-- spec/unit/rpc/client_spec.rb | 71 +++++++++------------ spec/unit/rpc/connectors/socket_spec.rb | 32 +++++++++- spec/unit/rpc/servers/socket_server_spec.rb | 20 +++--- 10 files changed, 130 insertions(+), 78 deletions(-) diff --git a/lib/protobuf/rpc/client.rb b/lib/protobuf/rpc/client.rb index e2cc56c6..3d25500b 100644 --- a/lib/protobuf/rpc/client.rb +++ b/lib/protobuf/rpc/client.rb @@ -9,7 +9,7 @@ class Client extend Forwardable include Protobuf::Logger::LogMethods - delegate [:options, :success_cb, :failure_cb, :async?] => :@connector + delegate [:options, :complete_cb, :success_cb, :failure_cb, :async?] => :@connector attr_reader :connector # Create a new client with default options (defined in ClientConnection) @@ -31,16 +31,19 @@ def initialize(opts={}) log_debug '[client] Initialized with options: %s' % opts.inspect end - # Set a success callback on the client to return the - # successful response from the service when it is returned. - # If this callback is called, failure_cb will NOT be called. + # Set a complete callback on the client to return the object (self). # Callback is called regardless of :async setting. # # client = Client.new(:service => WidgetService) - # client.on_success {|res| ... } + # client.on_complete {|obj| ... } # - def on_success(&success_cb) - @connector.success_cb = success_cb + def on_complete(&complete_cb) + @connector.complete_cb = complete_cb + end + + def on_complete=(callable) + raise "callable must take a single argument" if callable.arity != 1 + @connector.complete_cb = callable end # Set a failure callback on the client to return the @@ -54,6 +57,28 @@ def on_success(&success_cb) def on_failure(&failure_cb) @connector.failure_cb = failure_cb end + + def on_failure=(callable) + raise "callable must take a single argument" if callable.arity != 1 + @connector.failure_cb = callable + end + + # Set a success callback on the client to return the + # successful response from the service when it is returned. + # If this callback is called, failure_cb will NOT be called. + # Callback is called regardless of :async setting. + # + # client = Client.new(:service => WidgetService) + # client.on_success {|res| ... } + # + def on_success(&success_cb) + @connector.success_cb = success_cb + end + + def on_success=(callable) + raise "callable must take a single argument" if callable.arity != 1 + @connector.success_cb = callable + end # Provides a mechanism to call the service method against the client # which will automatically setup the service_class and method_name diff --git a/lib/protobuf/rpc/connectors/base.rb b/lib/protobuf/rpc/connectors/base.rb index 15720eb5..d0d7199f 100644 --- a/lib/protobuf/rpc/connectors/base.rb +++ b/lib/protobuf/rpc/connectors/base.rb @@ -24,7 +24,7 @@ class Base include Protobuf::Logger::LogMethods attr_reader :options - attr_accessor :success_cb, :failure_cb + attr_accessor :success_cb, :failure_cb, :complete_cb def initialize(options) @options = DEFAULT_OPTIONS.merge(options) diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index 274ad0e3..a5e876dd 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -10,12 +10,13 @@ class EMClient < EM::Connection attr_reader :options, :request, :response attr_reader :error, :error_reason, :error_message - class << self + class << self def connect(options={}) options = DEFAULT_OPTIONS.merge(options) Protobuf::Logger.debug '[client-cnxn] Connecting to server: %s' % options.inspect # Using 'attach' to get access to a Ruby socket if needed + # TODO use 'attach' and IO interface for plugin interface for new line protos socket = TCPSocket.new(options[:host], options[:port]) EM.attach(socket, self, socket, options) end diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index 55f3b8b2..7a2291a9 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -36,12 +36,8 @@ def ensure_cb private def ensure_em_running(&blk) - case - when EM.reactor_running? then + if EM.reactor_running? then yield - when async? then - @em_thread = Thread.new { EM.run(blk) } - Thread.pass until EM.reactor_running? else @em_thread = Thread.new { EM.fiber_run(blk) } Thread.pass until EM.reactor_running? diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index a0c1aa5c..daacdb50 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -9,9 +9,8 @@ class Socket < Base def send_request check_async initialize_stats - post_init connect_to_rpc_server - _send_request + post_init # calls _send_request read_response end diff --git a/lib/protobuf/rpc/service.rb b/lib/protobuf/rpc/service.rb index d68b29f5..2a544ba1 100644 --- a/lib/protobuf/rpc/service.rb +++ b/lib/protobuf/rpc/service.rb @@ -6,7 +6,7 @@ module Protobuf module Rpc # Object to encapsulate the request/response types for a given service method # - RpcMethod = Struct.new "RpcMethod", :service, :method, :request_type, :response_type + RpcMethod = Struct.new("RpcMethod", :service, :method, :request_type, :response_type) class Service include Protobuf::Logger::LogMethods @@ -65,7 +65,7 @@ def rpcs # See Client#initialize and ClientConnection::DEFAULT_OPTIONS # for all available options. # - def client options={} + def client(options={}) configure Client.new({ :service => self, @@ -80,7 +80,7 @@ def client options={} # so that any Clients using the Service.client sugar # will not have to configure the location each time. # - def configure config={} + def configure(config={}) locations[self] ||= {} locations[self][:host] = config[:host] if config.key?(:host) locations[self][:port] = config[:port] if config.key?(:port) @@ -90,7 +90,7 @@ def configure config={} # e.g. 127.0.0.1:9933 # e.g. localhost:0 # - def located_at location + def located_at(location) return if location.nil? or location.downcase.strip !~ /[a-z0-9.]+:\d+/ host, port = location.downcase.strip.split ':' configure :host => host, :port => port.to_i diff --git a/spec/helper/server.rb b/spec/helper/server.rb index 01f8e8fa..f40b6d9d 100644 --- a/spec/helper/server.rb +++ b/spec/helper/server.rb @@ -30,7 +30,10 @@ def post_init class StubServer include Protobuf::Logger::LogMethods + attr_accessor :options + def initialize(opts = {}) + @running = true @options = OpenStruct.new({ :host => "127.0.0.1", :port => 9939, @@ -38,29 +41,36 @@ def initialize(opts = {}) :server => Protobuf::Rpc::EventedServer }.merge(opts)) + start + yield self + ensure + stop if @running + end + + def start if @options.server == Protobuf::Rpc::EventedServer start_em_server else Protobuf::Rpc::SocketRunner.run(@options) end log_debug "[stub-server] Server started #{@options.host}:#{@options.port}" + rescue => ex + if ex =~ /no acceptor/ # Means EM didn't shutdown in the next_tick yet + stop + retry + end end def start_em_server - if !EM.reactor_running? - Thread.new { EM.run } - Thread.pass until EM.reactor_running? - end - @server_handle = EventMachine::start_server(@options.host, @options.port, StubProtobufServerFactory.build(@options.delay)) end def stop - case - when @options.server == Protobuf::Rpc::EventedServer then + if @options.server == Protobuf::Rpc::EventedServer EventMachine.stop_server(@server_handle) else Protobuf::Rpc::SocketRunner.stop end + @running = false end end diff --git a/spec/unit/rpc/client_spec.rb b/spec/unit/rpc/client_spec.rb index dc724063..07108f8d 100644 --- a/spec/unit/rpc/client_spec.rb +++ b/spec/unit/rpc/client_spec.rb @@ -6,55 +6,48 @@ context "when using fiber based calls" do it "waits for response when running synchronously" do EventMachine.fiber_run do - delay = 3 - server = StubServer.new(:delay => delay) - stop_servers = lambda { server.stop; EventMachine.stop } - client = Spec::Proto::TestService.client(:async => false) - - start = now - - client.find(:name => "Test Name", :active => true) do |c| - c.on_success do |succ| - succ.name.should eq("Test Name") - succ.status.should eq(Spec::Proto::StatusType::ENABLED) + StubServer.new(:delay => 3) do |server| + client = Spec::Proto::TestService.client(:async => false) + start = now + + client.find(:name => "Test Name", :active => true) do |c| + c.on_success do |succ| + succ.name.should eq("Test Name") + succ.status.should eq(Spec::Proto::StatusType::ENABLED) + end + + c.on_failure do |err| + raise err.inspect + end end - c.on_failure do |err| - raise err.inspect - end + (now - start).should be_within(server.options.delay * 0.10).of(server.options.delay) end - (now - start).should be_within(delay * 0.10).of(delay) - stop_servers.call + EM.stop end end it "doesn't wait for response when running async call inside fiber" do EventMachine.fiber_run do - delay = 3 - server = StubServer.new(:delay => delay) - stop_servers = lambda { server.stop; EventMachine.stop } - client = Spec::Proto::TestService.client(:async => true) - - start = now - - client.find(:name => "Test Name", :active => true) + StubServer.new(:delay => 3) do |server| + client = Spec::Proto::TestService.client(:async => true) + start = now + client.find(:name => "Test Name", :active => true) - (now - start).should_not be_within(delay * 0.10).of(delay) - stop_servers.call + (now - start).should_not be_within(server.options.delay* 0.10).of(server.options.delay) + end + EM.stop end end it "throws and error when synchronous code is attempted without 'EventMachine.fiber_run'" do subject = Proc.new do EventMachine.run do - delay = 1 - server = StubServer.new(:delay => delay) - stop_servers = lambda { server.stop; EventMachine.stop } - client = Spec::Proto::TestService.client(:async => false) - - client.find(:name => "Test Name", :active => true) - stop_servers.call + StubServer.new(:delay => 1) do |server| + client = Spec::Proto::TestService.client(:async => false) + client.find(:name => "Test Name", :active => true) + end end end @@ -64,13 +57,11 @@ it "throws a timeout when client timeout is exceeded" do subject = Proc.new do EventMachine.fiber_run do - delay = 3 - server = StubServer.new(:delay => delay) - stop_servers = lambda { server.stop; EventMachine.stop } - client = Spec::Proto::TestService.client(:async => false, :timeout => 1) - - client.find(:name => "Test Name", :active => true) - stop_servers.call + StubServer.new(:delay => 2) do |server| + client = Spec::Proto::TestService.client(:async => false, :timeout => 1) + client.find(:name => "Test Name", :active => true) + end + EM.stop end end diff --git a/spec/unit/rpc/connectors/socket_spec.rb b/spec/unit/rpc/connectors/socket_spec.rb index 1c0cd2de..ec757539 100644 --- a/spec/unit/rpc/connectors/socket_spec.rb +++ b/spec/unit/rpc/connectors/socket_spec.rb @@ -15,9 +15,39 @@ end describe Protobuf::Rpc::Connectors::Socket do + subject{ described_class.new({}) } + + it_behaves_like "a Protobuf Connector" specify{ described_class.include?(Protobuf::Rpc::Connectors::Common).should be_true } + context "#read_response" do + it "fills the buffer with data from the socket" do + data = "New data" + socket = StringIO.new("#{data.bytesize}-#{data}") + subject.instance_variable_set(:@buffer, Protobuf::Rpc::Buffer.new(:read)) + subject.instance_variable_set(:@socket, socket) + subject.should_receive(:parse_response).and_return(true) + + subject.__send__(:read_response) + subject.instance_variable_get(:@buffer).flushed?.should be_true + subject.instance_variable_get(:@buffer).data.should eq(data) + end + + it "waits for the IO to be readable" do + data = "New data" + socket = StringIO.new("#{data.bytesize}-#{data}") + slow_reader = OpenStruct.new(:read => lambda{ sleep 1; socket.read }.call ) + subject.instance_variable_set(:@buffer, Protobuf::Rpc::Buffer.new(:read)) + subject.instance_variable_set(:@socket, slow_reader) + subject.should_receive(:parse_response).and_return(true) + + subject.__send__(:read_response) + subject.instance_variable_get(:@buffer).flushed?.should be_true + subject.instance_variable_get(:@buffer).data.should eq(data) + end + end + context "#check_async" do it "raises an error when trying to execute asynchronously" do conn = described_class.new(:async => true) @@ -30,6 +60,4 @@ end end - it_behaves_like "a Protobuf Connector" - end diff --git a/spec/unit/rpc/servers/socket_server_spec.rb b/spec/unit/rpc/servers/socket_server_spec.rb index c577d8ab..7f5c01d2 100644 --- a/spec/unit/rpc/servers/socket_server_spec.rb +++ b/spec/unit/rpc/servers/socket_server_spec.rb @@ -10,7 +10,7 @@ end after(:all) do - sleep 5 + sleep 2 Protobuf::Rpc::SocketRunner.stop end @@ -30,16 +30,18 @@ context "Eventmachine client" do it "calls the service in the client request" do - client = Spec::Proto::TestService.client(:async => false, :port => 9399, :host => "127.0.0.1") + with_constants "Protobuf::ConnectorType" => "Evented" do + client = Spec::Proto::TestService.client(:async => false, :port => 9939, :host => "127.0.0.1") - client.find(:name => "Test Name", :active => true) do |c| - c.on_success do |succ| - succ.name.should eq("Test Name") - succ.status.should eq(Spec::Proto::StatusType::Enabled) - end + client.find(:name => "Test Name", :active => true) do |c| + c.on_success do |succ| + succ.name.should eq("Test Name") + succ.status.should eq(Spec::Proto::StatusType::Enabled) + end - c.on_failure do |err| - raise err.inspect + c.on_failure do |err| + raise err.inspect + end end end end From a4b94b5fefcdcb4aa0ecb2b7598aea1c140b867f Mon Sep 17 00:00:00 2001 From: = Date: Thu, 8 Dec 2011 15:11:50 -0700 Subject: [PATCH 32/50] use Thread.pass instead of sleep, and kill thread when done --- spec/unit/rpc/servers/socket_server_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/unit/rpc/servers/socket_server_spec.rb b/spec/unit/rpc/servers/socket_server_spec.rb index 7f5c01d2..b80470b7 100644 --- a/spec/unit/rpc/servers/socket_server_spec.rb +++ b/spec/unit/rpc/servers/socket_server_spec.rb @@ -5,13 +5,13 @@ describe Protobuf::Rpc::SocketServer do before(:all) do server = OpenStruct.new(:server => "127.0.0.1", :port => 9399) - Thread.new { Protobuf::Rpc::SocketRunner.run(server) } - sleep 0.5 # TODO figure out how to do this without sleep (Thread.pass didn't work either...hmmmm) + @server_thread = Thread.new { Protobuf::Rpc::SocketRunner.run(server) } + Thread.pass until Protobuf::Rpc::SocketServer.running? end after(:all) do - sleep 2 Protobuf::Rpc::SocketRunner.stop + Thread.kill(@server_thread) end it "provides a stop method" do From 467141cf1609532b52105ec90861d6ef58a8094e Mon Sep 17 00:00:00 2001 From: = Date: Thu, 8 Dec 2011 16:32:08 -0700 Subject: [PATCH 33/50] update spec --- spec/unit/rpc/connectors/socket_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/unit/rpc/connectors/socket_spec.rb b/spec/unit/rpc/connectors/socket_spec.rb index ec757539..f830fa27 100644 --- a/spec/unit/rpc/connectors/socket_spec.rb +++ b/spec/unit/rpc/connectors/socket_spec.rb @@ -22,8 +22,9 @@ specify{ described_class.include?(Protobuf::Rpc::Connectors::Common).should be_true } context "#read_response" do + let(:data){ "New data" } + it "fills the buffer with data from the socket" do - data = "New data" socket = StringIO.new("#{data.bytesize}-#{data}") subject.instance_variable_set(:@buffer, Protobuf::Rpc::Buffer.new(:read)) subject.instance_variable_set(:@socket, socket) @@ -35,7 +36,6 @@ end it "waits for the IO to be readable" do - data = "New data" socket = StringIO.new("#{data.bytesize}-#{data}") slow_reader = OpenStruct.new(:read => lambda{ sleep 1; socket.read }.call ) subject.instance_variable_set(:@buffer, Protobuf::Rpc::Buffer.new(:read)) From 2a32cfb57a93feeb146c35919164c63a714b7986 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 9 Dec 2011 14:42:08 -0700 Subject: [PATCH 34/50] run client inside current thread if EM is not running, should be able to start and stop EM as we see fit --- lib/protobuf/common/logger.rb | 4 ++++ lib/protobuf/ext/eventmachine.rb | 6 ++---- lib/protobuf/rpc/connectors/eventmachine.rb | 13 ++++++++---- lib/protobuf/rpc/connectors/socket.rb | 14 ++++++++++++- lib/protobuf/rpc/servers/socket_server.rb | 19 ++++++++++++++++- spec/unit/rpc/client_spec.rb | 2 +- spec/unit/rpc/connectors/socket_spec.rb | 14 ------------- spec/unit/rpc/servers/socket_server_spec.rb | 23 +-------------------- 8 files changed, 48 insertions(+), 47 deletions(-) diff --git a/lib/protobuf/common/logger.rb b/lib/protobuf/common/logger.rb index 54e3f08e..4ddeee49 100644 --- a/lib/protobuf/common/logger.rb +++ b/lib/protobuf/common/logger.rb @@ -52,6 +52,10 @@ module LogMethods Protobuf::Logger.__send__(m, *params, &block) end end + + def self.included(base) + base.extend(LogMethods) + end end end diff --git a/lib/protobuf/ext/eventmachine.rb b/lib/protobuf/ext/eventmachine.rb index 3ece8a2b..e49d986e 100644 --- a/lib/protobuf/ext/eventmachine.rb +++ b/lib/protobuf/ext/eventmachine.rb @@ -1,6 +1,6 @@ require 'fiber' -# Method and concept from em-synchrony +# Method from em-synchrony # https://github.com/igrigorik/em-synchrony # # A convenience method for wrapping EM.run body within @@ -8,9 +8,7 @@ # paused and resumed based on IO scheduling module EventMachine def self.fiber_run(blk=nil, tail=nil, &block) - blk ||= block - context = Proc.new{ Fiber.new{ blk.call }.resume } - + context = Proc.new{ Fiber.new{ (b = blk || block) and b.call }.resume } self.run(context, tail) end end diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index 7a2291a9..f0c0a0a2 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -7,7 +7,7 @@ module Connectors class EventMachine < Base def send_request - return ensure_em_running do + ensure_em_running do f = Fiber.current EM.schedule do @@ -36,11 +36,16 @@ def ensure_cb private def ensure_em_running(&blk) - if EM.reactor_running? then + if EM.reactor_running? yield else - @em_thread = Thread.new { EM.fiber_run(blk) } - Thread.pass until EM.reactor_running? + EM.fiber_run { + blk.call + EM.stop + } + + #Thread.pass until EM.reactor_running? + #EM.reactor_thread.join end end diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index daacdb50..7dd753e6 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -5,6 +5,7 @@ module Rpc module Connectors class Socket < Base include Protobuf::Rpc::Connectors::Common + include Protobuf::Logger::LogMethods def send_request check_async @@ -33,8 +34,19 @@ def error? @socket.closed? end + def read_data + size_io = StringIO.new + + while((size_reader = @socket.getc) != "-") + size_io << size_reader + end + str_size_io = size_io.string + + "#{str_size_io}-#{@socket.read(str_size_io.to_i)}" + end + def read_response - @buffer << @socket.read + @buffer << read_data parse_response if @buffer.flushed? end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index f121ff63..3e167cdc 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -15,11 +15,13 @@ def stop # TODO make listen/backlog part of config def run(host = "127.0.0.1", port = 9399, backlog = 100) + log_debug '[socket-server] Run' @running = true @server = TCPServer.new(host, port) @server.listen(backlog) while (sock = @server.accept) + log_debug '[socket-server] Accepted new connection' self.new(sock) sock.close end @@ -42,14 +44,29 @@ def initialize(sock) @response = Protobuf::Socketrpc::Response.new @buffer = Protobuf::Rpc::Buffer.new(:read) @stats = Protobuf::Rpc::Stat.new(:SERVER, true) + log_debug '[socket-server] Post init, new read buffer created' @stats.client = Socket.unpack_sockaddr_in(sock.getpeername) - @buffer << @socket.read + @buffer << read_data + log_debug '[socket-server] handling request' handle_client if @buffer.flushed? end + def read_data + size_io = StringIO.new + + while((size_reader = @socket.getc) != "-") + size_io << size_reader + end + str_size_io = size_io.string + + "#{str_size_io}-#{@socket.read(str_size_io.to_i)}" + end + def send_data(data) + log_debug '[socket-server] sending data : %s' % data @socket.write(data) + @socket.close_write end end diff --git a/spec/unit/rpc/client_spec.rb b/spec/unit/rpc/client_spec.rb index 07108f8d..b9c5f79b 100644 --- a/spec/unit/rpc/client_spec.rb +++ b/spec/unit/rpc/client_spec.rb @@ -154,7 +154,7 @@ client = Spec::Proto::TestService.client client.should_receive(:send_request) client.find({:name => 'Test Name', :active => false}) - client.options[:request].should be_a Spec::Proto::ResourceFindRequest + client.options[:request].should be_a(Spec::Proto::ResourceFindRequest) client.options[:request].name.should eq('Test Name') client.options[:request].active.should eq(false) end diff --git a/spec/unit/rpc/connectors/socket_spec.rb b/spec/unit/rpc/connectors/socket_spec.rb index f830fa27..4e7bd335 100644 --- a/spec/unit/rpc/connectors/socket_spec.rb +++ b/spec/unit/rpc/connectors/socket_spec.rb @@ -1,7 +1,6 @@ require 'spec_helper' shared_examples "a Protobuf Connector" do - subject{ described_class.new({}) } context "API" do @@ -11,7 +10,6 @@ specify{ subject.respond_to?(:close_connection, true).should be_true } specify{ subject.respond_to?(:error?, true).should be_true } end - end describe Protobuf::Rpc::Connectors::Socket do @@ -34,18 +32,6 @@ subject.instance_variable_get(:@buffer).flushed?.should be_true subject.instance_variable_get(:@buffer).data.should eq(data) end - - it "waits for the IO to be readable" do - socket = StringIO.new("#{data.bytesize}-#{data}") - slow_reader = OpenStruct.new(:read => lambda{ sleep 1; socket.read }.call ) - subject.instance_variable_set(:@buffer, Protobuf::Rpc::Buffer.new(:read)) - subject.instance_variable_set(:@socket, slow_reader) - subject.should_receive(:parse_response).and_return(true) - - subject.__send__(:read_response) - subject.instance_variable_get(:@buffer).flushed?.should be_true - subject.instance_variable_get(:@buffer).data.should eq(data) - end end context "#check_async" do diff --git a/spec/unit/rpc/servers/socket_server_spec.rb b/spec/unit/rpc/servers/socket_server_spec.rb index b80470b7..0bd49ae3 100644 --- a/spec/unit/rpc/servers/socket_server_spec.rb +++ b/spec/unit/rpc/servers/socket_server_spec.rb @@ -5,7 +5,7 @@ describe Protobuf::Rpc::SocketServer do before(:all) do server = OpenStruct.new(:server => "127.0.0.1", :port => 9399) - @server_thread = Thread.new { Protobuf::Rpc::SocketRunner.run(server) } + @server_thread = Thread.new(server) { |s| Protobuf::Rpc::SocketRunner.run(s) } Thread.pass until Protobuf::Rpc::SocketServer.running? end @@ -48,25 +48,4 @@ end - context 'when sending response objects' do - it 'should be able to send a hash object as a response' do - server = Protobuf::Rpc::SocketServer.new - - # Setup the right mocks - server.instance_variable_set(:@klass, Spec::Proto::TestService) - response_wrapper = mock('response') - response_wrapper.stub(:response_proto=) - server.instance_variable_set(:@response, response_wrapper) - Spec::Proto::TestService.stub_chain(:rpcs, :[], :[], :response_type).and_return(Spec::Proto::ResourceFindRequest) - - # Setup expectations - hash_response = {:name => 'Test Name', :active => false} - expected = Spec::Proto::ResourceFindRequest.new(hash_response) - Spec::Proto::ResourceFindRequest.should_receive(:new).with(hash_response).and_return(expected) - server.should_not_receive(:handle_error) - - # Call the method - server.send(:parse_response_from_service, hash_response) - end - end end From c8c1f27b661a3571156a280928fc55eefba2fe12 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 9 Dec 2011 15:46:28 -0700 Subject: [PATCH 35/50] dynamically startup EM and fix socket server spec --- lib/protobuf/rpc/connectors/eventmachine.rb | 14 ++------------ spec/unit/enum_spec.rb | 8 ++++---- spec/unit/rpc/servers/socket_server_spec.rb | 4 ++-- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index f0c0a0a2..712a12fa 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -9,7 +9,7 @@ class EventMachine < Base def send_request ensure_em_running do f = Fiber.current - + EM.schedule do log_debug '[client] Scheduling EventMachine client request to be created on next tick' cnxn = EMClient.connect(options, &ensure_cb) @@ -36,17 +36,7 @@ def ensure_cb private def ensure_em_running(&blk) - if EM.reactor_running? - yield - else - EM.fiber_run { - blk.call - EM.stop - } - - #Thread.pass until EM.reactor_running? - #EM.reactor_thread.join - end + EM.reactor_running? ? yield : EM.fiber_run { blk.call; EM.stop } end def resume_fiber(fib) diff --git a/spec/unit/enum_spec.rb b/spec/unit/enum_spec.rb index 3796dd0b..2a5f337f 100644 --- a/spec/unit/enum_spec.rb +++ b/spec/unit/enum_spec.rb @@ -4,10 +4,10 @@ describe Protobuf::Enum do context 'when coercing from enum' do subject { Spec::Proto::StatusType::PENDING } - it { should == 0 } + it { should eq(0) } end + context 'when coercing from integer' do - subject { 0 } - it { should == Spec::Proto::StatusType::PENDING } + it { 0.should eq(Spec::Proto::StatusType::PENDING) } end -end \ No newline at end of file +end diff --git a/spec/unit/rpc/servers/socket_server_spec.rb b/spec/unit/rpc/servers/socket_server_spec.rb index 0bd49ae3..18ce6d72 100644 --- a/spec/unit/rpc/servers/socket_server_spec.rb +++ b/spec/unit/rpc/servers/socket_server_spec.rb @@ -31,12 +31,12 @@ it "calls the service in the client request" do with_constants "Protobuf::ConnectorType" => "Evented" do - client = Spec::Proto::TestService.client(:async => false, :port => 9939, :host => "127.0.0.1") + client = Spec::Proto::TestService.client(:async => false, :port => 9399, :host => "127.0.0.1") client.find(:name => "Test Name", :active => true) do |c| c.on_success do |succ| succ.name.should eq("Test Name") - succ.status.should eq(Spec::Proto::StatusType::Enabled) + succ.status.should eq(Spec::Proto::StatusType::ENABLED) end c.on_failure do |err| From 09f649773a563c42508cf5939e486c8b52504272 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 9 Dec 2011 18:52:36 -0700 Subject: [PATCH 36/50] add debug logging to socket connector class --- lib/protobuf/rpc/connectors/socket.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index 7dd753e6..0e480fa7 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -18,20 +18,28 @@ def send_request private def check_async - raise "Cannot use Socket client in async mode" if async? + if async? + log_error "[client-#{self.class}] Cannot run in async mode" + raise "Cannot use Socket client in async mode" + else + log_debug "[client-#{self.class}] Async check passed" + end end def close_connection @socket.close + log_debug "[client-#{self.class}] Connector closed" end def connect_to_rpc_server @socket = TCPSocket.new(options[:host], options[:port]) + log_debug "[client-#{self.class}] Connection established #{options[:host]}:#{options[:port]}" end # Method to determine error state, must be used with Connector api def error? @socket.closed? + log_debug "[client-#{self.class}] Error state : #{@socket.closed?}" end def read_data @@ -53,6 +61,7 @@ def read_response def send_data(data) @socket.write(data) @socket.close_write + log_debug "[client-#{self.class}] Write closed." end end From 884052e459bb3d4ec0d20bd2446ce1c423617b2b Mon Sep 17 00:00:00 2001 From: = Date: Fri, 9 Dec 2011 19:04:57 -0700 Subject: [PATCH 37/50] removing stdout logging --- lib/protobuf/rpc/servers/evented_runner.rb | 1 - lib/protobuf/rpc/servers/socket_runner.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/lib/protobuf/rpc/servers/evented_runner.rb b/lib/protobuf/rpc/servers/evented_runner.rb index 3add6e89..e72b0878 100644 --- a/lib/protobuf/rpc/servers/evented_runner.rb +++ b/lib/protobuf/rpc/servers/evented_runner.rb @@ -7,7 +7,6 @@ class << self def stop EventMachine.stop_event_loop if EventMachine.reactor_running? Protobuf::Logger.info 'Shutdown complete' - $stdout.puts 'Shutdown complete' end def run(server) diff --git a/lib/protobuf/rpc/servers/socket_runner.rb b/lib/protobuf/rpc/servers/socket_runner.rb index 70f629c7..81f4f846 100644 --- a/lib/protobuf/rpc/servers/socket_runner.rb +++ b/lib/protobuf/rpc/servers/socket_runner.rb @@ -6,7 +6,6 @@ class << self def stop Protobuf::Rpc::SocketServer.stop Protobuf::Logger.info 'Shutdown complete' - $stdout.puts 'Shutdown complete' end def run(server) From ad17b28b711f44dd48108e3966db25d70b498a71 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 9 Dec 2011 23:17:54 -0700 Subject: [PATCH 38/50] add threaded execution on socket server and threadpool/thresholds --- lib/protobuf.rb | 2 ++ lib/protobuf/message/message.rb | 4 +-- lib/protobuf/rpc/client.rb | 15 ++++++-- lib/protobuf/rpc/connectors/common.rb | 36 +++++++++++-------- lib/protobuf/rpc/server.rb | 24 +++++++------ lib/protobuf/rpc/servers/socket_server.rb | 43 ++++++++++++++++------- 6 files changed, 80 insertions(+), 44 deletions(-) diff --git a/lib/protobuf.rb b/lib/protobuf.rb index 8da1b63c..91d4c9bd 100644 --- a/lib/protobuf.rb +++ b/lib/protobuf.rb @@ -1,5 +1,7 @@ require 'logger' require 'socket' +require 'pp' +require 'stringio' module Protobuf end diff --git a/lib/protobuf/message/message.rb b/lib/protobuf/message/message.rb index 8a018f40..d06e41d7 100644 --- a/lib/protobuf/message/message.rb +++ b/lib/protobuf/message/message.rb @@ -1,5 +1,3 @@ -require 'pp' -require 'stringio' require 'protobuf/descriptor/descriptor' require 'protobuf/message/decoder' require 'protobuf/message/encoder' @@ -27,7 +25,7 @@ def include_tag?(tag) end end - class < error # Ensure we're handling any errors that try to slip out the back door @@ -37,7 +37,7 @@ def handle_client # Client error handler. Receives an exception object and writes it into the @response def handle_error(error) - log_debug '[server] handle_error: %s' % error.inspect + log_debug "[#{log_signature}] handle_error: %s" % error.inspect if error.respond_to?(:to_response) error.to_response(@response) else @@ -73,14 +73,18 @@ def invoke_rpc_method end # Call the service method - log_debug '[server] Invoking %s#%s with request %s' % [@klass.name, @method, @request.inspect] + log_debug "[#{log_signature}] Invoking %s#%s with request %s" % [@klass.name, @method, @request.inspect] @service.__send__(@method, @request) end + + def log_signature + @log_signature ||= "server-#{self.class}" + end # Parse the incoming request object into our expected request object def parse_request_from_buffer - log_debug '[server] parsing request from buffer: %s' % @buffer.data.inspect - @request.parse_from_string @buffer.data + log_debug "[#{log_signature}] parsing request from buffer: %s" % @buffer.data.inspect + @request.parse_from_string(@buffer.data) rescue => error exc = BadRequestData.new 'Unable to parse request: %s' % error.message log_error exc.message @@ -96,7 +100,7 @@ def parse_response_from_service(response) # Cannibalize the response if it's a Hash response = expected.new(response) if response.is_a?(Hash) actual = response.class - log_debug '[server] response (should/actual): %s/%s' % [expected.name, actual.name] + log_debug "[#{log_signature}] response (should/actual): %s/%s" % [expected.name, actual.name] # Determine if the service tried to change response types on us if expected == actual @@ -129,7 +133,7 @@ def parse_service_info # Write the response wrapper to the client def send_response raise 'Response already sent to client' if @did_respond - log_debug '[server] Sending response to client: %s' % @response.inspect + log_debug "[#{log_signature}] Sending response to client: %s" % @response.inspect response_buffer = Protobuf::Rpc::Buffer.new(:write, @response) send_data(response_buffer.write) @stats.response_size = response_buffer.size @@ -139,7 +143,7 @@ def send_response end def serialize_response(response) - log_debug '[server] serializing response: %s' % response.inspect + log_debug "[#{log_signature}] serializing response: %s" % response.inspect @response.response_proto = response.serialize_to_string rescue raise BadResponseProto, $!.message diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index 3e167cdc..b3c75b07 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -8,22 +8,34 @@ class SocketServer class << self - def stop - @running = false - @server.close + def cleanup? + # every 10 connections run a cleanup routine after closing the response + @threads.size > (@thread_threshold - 1) && (@threads.size % @thread_threshold) == 0 + end + + def cleanup_threads + log_debug "[socket-#{self}] Thread cleanup - #{@threads.size} - start" + @threads = @threads.select{ |t| t.alive? ? true : (t.join; false) } + log_debug "[socket-#{self}] Thread cleanup - #{@threads.size} - complete" end # TODO make listen/backlog part of config - def run(host = "127.0.0.1", port = 9399, backlog = 100) - log_debug '[socket-server] Run' + def run(host = "127.0.0.1", port = 9399, backlog = 100, thread_threshold = 10) + log_debug "[socket-#{self}] Run" @running = true + @threads = [] + @thread_threshold = thread_threshold @server = TCPServer.new(host, port) @server.listen(backlog) - while (sock = @server.accept) - log_debug '[socket-server] Accepted new connection' - self.new(sock) - sock.close + while true + @threads << Thread.new(@server.accept) do |sock| + log_debug "[socket-#{self}] Accepted new connection" + Protobuf::Rpc::SocketServer.new(sock) + sock.close + end + + cleanup_threads if cleanup? end rescue Errno::EBADF @@ -35,6 +47,11 @@ def running? @running end + def stop + @running = false + @server.close + end + end def initialize(sock) @@ -44,11 +61,11 @@ def initialize(sock) @response = Protobuf::Socketrpc::Response.new @buffer = Protobuf::Rpc::Buffer.new(:read) @stats = Protobuf::Rpc::Stat.new(:SERVER, true) - log_debug '[socket-server] Post init, new read buffer created' + log_debug "[server-#{self.class}] Post init, new read buffer created" - @stats.client = Socket.unpack_sockaddr_in(sock.getpeername) + @stats.client = Socket.unpack_sockaddr_in(@socket.getpeername) @buffer << read_data - log_debug '[socket-server] handling request' + log_debug "[server-#{self.class}] handling request" handle_client if @buffer.flushed? end @@ -64,7 +81,7 @@ def read_data end def send_data(data) - log_debug '[socket-server] sending data : %s' % data + log_debug "[server-#{self.class}] sending data : %s" % data @socket.write(data) @socket.close_write end From 9ae1945f1f74341e7b76750adc2b124b9d7766fc Mon Sep 17 00:00:00 2001 From: = Date: Sat, 10 Dec 2011 12:11:00 -0700 Subject: [PATCH 39/50] adding logging signatures to connectors/servers --- lib/protobuf/rpc/connectors/em_client.rb | 10 ++++------ lib/protobuf/rpc/connectors/eventmachine.rb | 10 +++++++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index a5e876dd..5c737db5 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -14,7 +14,7 @@ class << self def connect(options={}) options = DEFAULT_OPTIONS.merge(options) - Protobuf::Logger.debug '[client-cnxn] Connecting to server: %s' % options.inspect + log_debug "[client-#{self}] Connecting to server: %s" % options.inspect # Using 'attach' to get access to a Ruby socket if needed # TODO use 'attach' and IO interface for plugin interface for new line protos socket = TCPSocket.new(options[:host], options[:port]) @@ -28,11 +28,9 @@ def initialize(socket, options={}, &failure_cb) @failure_cb = failure_cb @options = DEFAULT_OPTIONS.merge(options) verify_options - - log_debug '[client-cnxn] Client Initialized: %s' % options.inspect - @success_cb = nil - initialize_stats + + log_debug "[#{log_signature}] Client Initialized: %s" % options.inspect rescue fail(:RPC_ERROR, 'Failed to initialize connection: %s' % $!.message) end @@ -53,7 +51,7 @@ def on_complete(&complete_cb) end def receive_data(data) - log_debug '[client-cnxn] receive_data: %s' % data + log_debug "[#{log_signature}] receive_data: %s" % data @buffer << data parse_response if @buffer.flushed? end diff --git a/lib/protobuf/rpc/connectors/eventmachine.rb b/lib/protobuf/rpc/connectors/eventmachine.rb index 712a12fa..6a42fe4d 100644 --- a/lib/protobuf/rpc/connectors/eventmachine.rb +++ b/lib/protobuf/rpc/connectors/eventmachine.rb @@ -11,12 +11,12 @@ def send_request f = Fiber.current EM.schedule do - log_debug '[client] Scheduling EventMachine client request to be created on next tick' + log_debug "[#{log_signature}] Scheduling EventMachine client request to be created on next tick" cnxn = EMClient.connect(options, &ensure_cb) cnxn.on_success(&success_cb) if success_cb cnxn.on_failure(&ensure_cb) cnxn.on_complete { resume_fiber(f) } unless async? - log_debug '[client] Connection scheduled' + log_debug "[#{log_signature}] Connection scheduled" end async? ? true : set_timeout_and_validate_fiber @@ -32,6 +32,10 @@ def send_request def ensure_cb @ensure_cb ||= (@failure_cb || lambda { |error| raise '%s: %s' % [error.code.name, error.message] } ) end + + def log_signature + @log_signature ||= "client-#{self.class}" + end private @@ -43,7 +47,7 @@ def resume_fiber(fib) EM::cancel_timer(@timeout_timer) fib.resume(true) rescue => ex - log_error 'An exception occurred while waiting for server response:' + log_error "[#{log_signature}] An exception occurred while waiting for server response:" log_error ex.message log_error ex.backtrace.join("\n") From c5c2ff28946de553133c6737a343452d900475be Mon Sep 17 00:00:00 2001 From: = Date: Sat, 10 Dec 2011 12:58:35 -0700 Subject: [PATCH 40/50] align log_signature methods --- lib/protobuf/rpc/client.rb | 20 ++++++++++++-------- lib/protobuf/rpc/service.rb | 18 +++++++++++------- lib/protobuf/rpc/stat.rb | 2 +- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/protobuf/rpc/client.rb b/lib/protobuf/rpc/client.rb index b7796af1..d867e3a3 100644 --- a/lib/protobuf/rpc/client.rb +++ b/lib/protobuf/rpc/client.rb @@ -28,7 +28,11 @@ class Client def initialize(opts={}) raise "Invalid client configuration. Service must be defined." if opts[:service].nil? @connector = Connector.connector_for_client.new(opts) - log_debug '[client] Initialized with options: %s' % opts.inspect + log_debug "[#{log_signature}] Initialized with options: %s" % opts.inspect + end + + def log_signature + @log_signature ||= "client-#{self.class}" end # Set a complete callback on the client to return the object (self). @@ -104,25 +108,25 @@ def on_success=(callable) def method_missing(method, *params) service = options[:service] unless service.rpcs[service].keys.include?(method) - log_error '[client] %s#%s not rpc method, passing to super' % [service.name, method.to_s] + log_error "[#{log_signature}] %s#%s not rpc method, passing to super" % [service.name, method.to_s] super(method, *params) else - log_debug '[client] %s#%s' % [service.name, method.to_s] + log_debug "[#{log_signature}] %s#%s" % [service.name, method.to_s] rpc = service.rpcs[service][method.to_sym] options[:request_type] = rpc.request_type - log_debug '[client] Request Type: %s' % options[:request_type].name + log_debug "[#{log_signature}] Request Type: %s" % options[:request_type].name options[:response_type] = rpc.response_type - log_debug '[client] Response Type: %s' % options[:response_type].name + log_debug "[#{log_signature}] Response Type: %s" % options[:response_type].name options[:method] = method.to_s options[:request] = params[0].is_a?(Hash) ? options[:request_type].new(params[0]) : params[0] - log_debug '[client] Request Data: %s' % options[:request].inspect + log_debug "[#{log_signature}] Request Data: %s" % options[:request].inspect # Call client to setup on_success and on_failure event callbacks if block_given? - log_debug '[client] client setup callback given, invoking' + log_debug "[#{log_signature}] client setup callback given, invoking" yield(self) else - log_debug '[client] no block given for callbacks' + log_debug "[#{log_signature}] no block given for callbacks" end send_request diff --git a/lib/protobuf/rpc/service.rb b/lib/protobuf/rpc/service.rb index 2a544ba1..be887461 100644 --- a/lib/protobuf/rpc/service.rb +++ b/lib/protobuf/rpc/service.rb @@ -51,7 +51,7 @@ def method_added(old) # Generated service classes should call this method on themselves to add rpc methods # to the stack with a given request and response type - def rpc method, request_type, response_type + def rpc(method, request_type, response_type) rpcs[self] ||= {} rpcs[self][method] = RpcMethod.new self, method, request_type, response_type end @@ -114,6 +114,10 @@ def locations end end + + def log_signature + @log_signature ||= "service-#{self.class}" + end # If a method comes through that hasn't been found, and it # is defined in the rpcs method list, we know that the rpc @@ -126,7 +130,7 @@ def method_missing m, *params log_error exc.message raise exc else - log_error '-------------- [service] %s#%s not rpc method, passing to super' % [self.class.name, m.to_s] + log_error "-------------- [#{log_signature}] %s#%s not rpc method, passing to super" % [self.class.name, m.to_s] super m, params end end @@ -150,7 +154,7 @@ def rpc_failed(message="RPC Failed while executing service method #{@current_met error_message = 'Unable to invoke rpc_failed, no failure callback is setup.' log_and_raise_error(error_message) if @rpc_failure_cb.nil? error = message.is_a?(String) ? RpcFailed.new(message) : message - log_warn '[service] RPC Failed: %s' % error.message + log_warn "[#{log_signature}] RPC Failed: %s" % error.message @rpc_failure_cb.call(error) end @@ -217,17 +221,17 @@ def call_rpc(method, pb_request) # Setup the response @response = rpcs[method].response_type.new - log_debug '[service] calling service method %s#%s' % [self.class.name, method] + log_debug "[#{log_signature}] calling service method %s#%s" % [self.class.name, method] # Call the aliased rpc method (e.g. :rpc_find for :find) __send__("rpc_#{method}".to_sym) - log_debug '[service] completed service method %s#%s' % [self.class.name, method] + log_debug "[#{log_signature}] completed service method %s#%s" % [self.class.name, method] # Pass the populated response back to the server # Note this will only get called if the rpc method didn't explode (by design) if @async_responder - log_debug '[service] async request, not sending response (yet)' + log_debug "[#{log_signature}] async request, not sending response (yet)" else - log_debug '[service] trigger server send_response' + log_debug "[#{log_signature}] trigger server send_response" send_response end end diff --git a/lib/protobuf/rpc/stat.rb b/lib/protobuf/rpc/stat.rb index 6314dedf..c4ba56f5 100644 --- a/lib/protobuf/rpc/stat.rb +++ b/lib/protobuf/rpc/stat.rb @@ -56,7 +56,7 @@ def log_stats def to_s [ - @type == :SERVER ? '[SRV]' : '[CLT]', + @type == :SERVER ? "[SRV-#{self.class}]" : "[CLT-#{self.class}]", rpc, elapsed_time, sizes, From f386ca55e3d9d27f5a85ff3d2bfc3908e02e4d9c Mon Sep 17 00:00:00 2001 From: = Date: Sat, 10 Dec 2011 13:40:27 -0700 Subject: [PATCH 41/50] mark log_signature as NON_RPC --- lib/protobuf/rpc/service.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/protobuf/rpc/service.rb b/lib/protobuf/rpc/service.rb index be887461..05d2d920 100644 --- a/lib/protobuf/rpc/service.rb +++ b/lib/protobuf/rpc/service.rb @@ -26,7 +26,7 @@ class << self # You MUST add the method name to this list if you are adding # instance methods below, otherwise stuff will definitely break - NON_RPC_METHODS = %w( rpcs call_rpc on_rpc_failed rpc_failed request response method_missing async_responder on_send_response send_response ) + NON_RPC_METHODS = %w( rpcs call_rpc on_rpc_failed rpc_failed request response method_missing async_responder on_send_response send_response log_signature ) # Override methods being added to the class # If the method isn't already a private instance method, or it doesn't start with rpc_, @@ -41,7 +41,7 @@ def method_added(old) private new_method define_method(old) do |pb_request| - call_rpc old.to_sym, pb_request + call_rpc(old.to_sym, pb_request) end rescue ArgumentError => e # Wrap a known issue where an instance method was defined in the class without @@ -221,10 +221,10 @@ def call_rpc(method, pb_request) # Setup the response @response = rpcs[method].response_type.new - log_debug "[#{log_signature}] calling service method %s#%s" % [self.class.name, method] + log_debug "[#{log_signature}] calling service method %s#%s" % [self.class, method] # Call the aliased rpc method (e.g. :rpc_find for :find) __send__("rpc_#{method}".to_sym) - log_debug "[#{log_signature}] completed service method %s#%s" % [self.class.name, method] + log_debug "[#{log_signature}] completed service method %s#%s" % [self.class, method] # Pass the populated response back to the server # Note this will only get called if the rpc method didn't explode (by design) From 293c6368dc430d02cbdba3bb7039de6e7f5e7475 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 10 Dec 2011 13:47:16 -0700 Subject: [PATCH 42/50] make the accepting socket object self-contained (a Worker) --- lib/protobuf/rpc/servers/socket_server.rb | 60 ++++++++++++----------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index b3c75b07..db816812 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -31,7 +31,7 @@ def run(host = "127.0.0.1", port = 9399, backlog = 100, thread_threshold = 10) while true @threads << Thread.new(@server.accept) do |sock| log_debug "[socket-#{self}] Accepted new connection" - Protobuf::Rpc::SocketServer.new(sock) + Protobuf::Rpc::SocketServer::Worker.new(sock) sock.close end @@ -54,38 +54,42 @@ def stop end - def initialize(sock) - @did_response = false - @socket = sock - @request = Protobuf::Socketrpc::Request.new - @response = Protobuf::Socketrpc::Response.new - @buffer = Protobuf::Rpc::Buffer.new(:read) - @stats = Protobuf::Rpc::Stat.new(:SERVER, true) - log_debug "[server-#{self.class}] Post init, new read buffer created" - - @stats.client = Socket.unpack_sockaddr_in(@socket.getpeername) - @buffer << read_data - log_debug "[server-#{self.class}] handling request" - handle_client if @buffer.flushed? - end + class Worker + include Protobuf::Rpc::Server + include Protobuf::Logger::LogMethods + + def initialize(sock) + @did_response = false + @socket = sock + @request = Protobuf::Socketrpc::Request.new + @response = Protobuf::Socketrpc::Response.new + @buffer = Protobuf::Rpc::Buffer.new(:read) + @stats = Protobuf::Rpc::Stat.new(:SERVER, true) + log_debug "[server-#{self.class}] Post init, new read buffer created" + + @stats.client = Socket.unpack_sockaddr_in(@socket.getpeername) + @buffer << read_data + log_debug "[server-#{self.class}] handling request" + handle_client if @buffer.flushed? + end - def read_data - size_io = StringIO.new + def read_data + size_io = StringIO.new - while((size_reader = @socket.getc) != "-") - size_io << size_reader - end - str_size_io = size_io.string + while((size_reader = @socket.getc) != "-") + size_io << size_reader + end + str_size_io = size_io.string - "#{str_size_io}-#{@socket.read(str_size_io.to_i)}" - end + "#{str_size_io}-#{@socket.read(str_size_io.to_i)}" + end - def send_data(data) - log_debug "[server-#{self.class}] sending data : %s" % data - @socket.write(data) - @socket.close_write + def send_data(data) + log_debug "[server-#{self.class}] sending data : %s" % data + @socket.write(data) + @socket.close_write + end end - end end end From d746f6b0f5969eb2ba5714f923a5f3dae7e051dd Mon Sep 17 00:00:00 2001 From: = Date: Sat, 10 Dec 2011 13:50:05 -0700 Subject: [PATCH 43/50] std log_signature methods --- lib/protobuf/rpc/servers/socket_server.rb | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index db816812..51c358f2 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -14,14 +14,18 @@ def cleanup? end def cleanup_threads - log_debug "[socket-#{self}] Thread cleanup - #{@threads.size} - start" + log_debug "[#{log_signature}] Thread cleanup - #{@threads.size} - start" @threads = @threads.select{ |t| t.alive? ? true : (t.join; false) } - log_debug "[socket-#{self}] Thread cleanup - #{@threads.size} - complete" + log_debug "[#{log_signature}] Thread cleanup - #{@threads.size} - complete" + end + + def log_signature + @log_signature ||= "server-#{self}" end # TODO make listen/backlog part of config def run(host = "127.0.0.1", port = 9399, backlog = 100, thread_threshold = 10) - log_debug "[socket-#{self}] Run" + log_debug "[#{log_signature}] Run" @running = true @threads = [] @thread_threshold = thread_threshold @@ -30,7 +34,7 @@ def run(host = "127.0.0.1", port = 9399, backlog = 100, thread_threshold = 10) while true @threads << Thread.new(@server.accept) do |sock| - log_debug "[socket-#{self}] Accepted new connection" + log_debug "[#{log_signature}] Accepted new connection" Protobuf::Rpc::SocketServer::Worker.new(sock) sock.close end @@ -65,14 +69,18 @@ def initialize(sock) @response = Protobuf::Socketrpc::Response.new @buffer = Protobuf::Rpc::Buffer.new(:read) @stats = Protobuf::Rpc::Stat.new(:SERVER, true) - log_debug "[server-#{self.class}] Post init, new read buffer created" + log_debug "[#{log_signature}] Post init, new read buffer created" @stats.client = Socket.unpack_sockaddr_in(@socket.getpeername) @buffer << read_data - log_debug "[server-#{self.class}] handling request" + log_debug "[#{log_signature}] handling request" handle_client if @buffer.flushed? end + def log_signature + @log_signature ||= "server-#{self.class}" + end + def read_data size_io = StringIO.new @@ -85,7 +93,7 @@ def read_data end def send_data(data) - log_debug "[server-#{self.class}] sending data : %s" % data + log_debug "[#{log_signature}] sending data : %s" % data @socket.write(data) @socket.close_write end From c2bbdcc491e7025288528e5545108df08e4a2ec3 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 10 Dec 2011 14:17:29 -0700 Subject: [PATCH 44/50] switch back to letting EM do attach, need to figure out issues with attach manual (only allowig 40 fds at a time) --- lib/protobuf/rpc/connectors/em_client.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/protobuf/rpc/connectors/em_client.rb b/lib/protobuf/rpc/connectors/em_client.rb index 5c737db5..eb7c5cfe 100644 --- a/lib/protobuf/rpc/connectors/em_client.rb +++ b/lib/protobuf/rpc/connectors/em_client.rb @@ -15,16 +15,12 @@ class << self def connect(options={}) options = DEFAULT_OPTIONS.merge(options) log_debug "[client-#{self}] Connecting to server: %s" % options.inspect - # Using 'attach' to get access to a Ruby socket if needed - # TODO use 'attach' and IO interface for plugin interface for new line protos - socket = TCPSocket.new(options[:host], options[:port]) - EM.attach(socket, self, socket, options) + EM.connect(options[:host], options[:port], self, options) end end - def initialize(socket, options={}, &failure_cb) - @socket = socket + def initialize(options={}, &failure_cb) @failure_cb = failure_cb @options = DEFAULT_OPTIONS.merge(options) verify_options From 374e4ab817c07aad075bbfc1e2ca9c286fc228ab Mon Sep 17 00:00:00 2001 From: = Date: Sat, 10 Dec 2011 22:42:23 -0700 Subject: [PATCH 45/50] add thread mngr to socket srvr and java specific stuff --- bin/rpc_server | 2 +- lib/protobuf/rpc/connector.rb | 3 - lib/protobuf/rpc/connectors/socket.rb | 5 +- lib/protobuf/rpc/servers/socket_server.rb | 84 +++++++++++++++++++---- 4 files changed, 74 insertions(+), 20 deletions(-) diff --git a/bin/rpc_server b/bin/rpc_server index 87ca7d5c..3a6f65c3 100755 --- a/bin/rpc_server +++ b/bin/rpc_server @@ -102,7 +102,7 @@ begin server.runner.run(server) rescue - msg = 'ERROR: RPC Server failed to start. %s' % $!.message + msg = 'ERROR: RPC Server failed to start. %s' % $!.inspect $stderr.puts msg, *($!.backtrace) Protobuf::Logger.error msg Protobuf::Logger.error $!.backtrace.join("\n") diff --git a/lib/protobuf/rpc/connector.rb b/lib/protobuf/rpc/connector.rb index 8108026f..e2762705 100644 --- a/lib/protobuf/rpc/connector.rb +++ b/lib/protobuf/rpc/connector.rb @@ -1,6 +1,3 @@ -require 'protobuf/rpc/connectors/eventmachine' -require 'protobuf/rpc/connectors/socket' - module Protobuf module Rpc class Connector diff --git a/lib/protobuf/rpc/connectors/socket.rb b/lib/protobuf/rpc/connectors/socket.rb index 0e480fa7..9e0be3b6 100644 --- a/lib/protobuf/rpc/connectors/socket.rb +++ b/lib/protobuf/rpc/connectors/socket.rb @@ -38,8 +38,8 @@ def connect_to_rpc_server # Method to determine error state, must be used with Connector api def error? - @socket.closed? log_debug "[client-#{self.class}] Error state : #{@socket.closed?}" + @socket.closed? end def read_data @@ -60,8 +60,9 @@ def read_response def send_data(data) @socket.write(data) + @socket.flush @socket.close_write - log_debug "[client-#{self.class}] Write closed." + log_debug "[client-#{self.class}] write closed" end end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index 51c358f2..1999020e 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -15,7 +15,17 @@ def cleanup? def cleanup_threads log_debug "[#{log_signature}] Thread cleanup - #{@threads.size} - start" - @threads = @threads.select{ |t| t.alive? ? true : (t.join; false) } + + @threads = @threads.select do |t| + if t[:thread].alive? + true + else + t[:thread].join + @working.delete(t[:socket]) + false + end + end + log_debug "[#{log_signature}] Thread cleanup - #{@threads.size} - complete" end @@ -23,26 +33,70 @@ def log_signature @log_signature ||= "server-#{self}" end + # TODO fiber way to get a new worker? + def new_worker(socket) + Thread.new(socket) do |sock| + Protobuf::Rpc::SocketServer::Worker.new(sock) do |s| + s.close + end + end + end + # TODO make listen/backlog part of config - def run(host = "127.0.0.1", port = 9399, backlog = 100, thread_threshold = 10) + def run(host = "127.0.0.1", port = 9399, backlog = 100, thread_threshold = 100) log_debug "[#{log_signature}] Run" @running = true @threads = [] @thread_threshold = thread_threshold @server = TCPServer.new(host, port) @server.listen(backlog) - - while true - @threads << Thread.new(@server.accept) do |sock| - log_debug "[#{log_signature}] Accepted new connection" - Protobuf::Rpc::SocketServer::Worker.new(sock) - sock.close + @working = [] + @listen_fds = [@server] + + while running? + log_debug "[#{log_signature}] Waiting for connections" + + if ready_cnxns = IO.select(@listen_fds, [], [], 20) + cnxns = ready_cnxns.first + cnxns.each do |client| + case + when !running? then + # no-op + when client == @server then + log_debug "[#{log_signature}] Accepted new connection" + client, sockaddr = @server.accept + log_debug "--------------->#{sockaddr}" + @listen_fds << client + else + if !@working.include?(client) + @listen_fds.delete(client) + @working << client + log_debug "[#{log_signature}] Working" + @threads << { :thread => new_worker(client), + :socket => client } + + cleanup_threads if cleanup? + end + end + end + else + # Run a cleanup if select times out while waiting + cleanup_threads if @threads.size > 1 end - - cleanup_threads if cleanup? end - rescue Errno::EBADF +# # Threaded server +# while true +# @threads << Thread.new(@server.accept) do |sock| +# log_debug "[#{log_signature}] Accepted new connection" +# Protobuf::Rpc::SocketServer::Worker.new(sock) +# sock.close +# end +# +# cleanup_threads if cleanup? +# end + + rescue # Closing the server causes the loop to raise an exception here raise if running? end @@ -62,13 +116,14 @@ class Worker include Protobuf::Rpc::Server include Protobuf::Logger::LogMethods - def initialize(sock) + def initialize(sock, &complete_cb) @did_response = false @socket = sock @request = Protobuf::Socketrpc::Request.new @response = Protobuf::Socketrpc::Response.new @buffer = Protobuf::Rpc::Buffer.new(:read) @stats = Protobuf::Rpc::Stat.new(:SERVER, true) + @complete_cb = complete_cb log_debug "[#{log_signature}] Post init, new read buffer created" @stats.client = Socket.unpack_sockaddr_in(@socket.getpeername) @@ -78,7 +133,7 @@ def initialize(sock) end def log_signature - @log_signature ||= "server-#{self.class}" + @log_signature ||= "server-#{self.class}-#{object_id}" end def read_data @@ -95,7 +150,8 @@ def read_data def send_data(data) log_debug "[#{log_signature}] sending data : %s" % data @socket.write(data) - @socket.close_write + @socket.flush + @complete_cb.call(@socket) end end end From 6f16fa88ccce8cc7e9acb654d9196a0fac6a376d Mon Sep 17 00:00:00 2001 From: = Date: Wed, 14 Dec 2011 01:35:17 -0700 Subject: [PATCH 46/50] clean up specs --- Gemfile.lock | 5 ++- lib/protobuf.rb | 1 + lib/protobuf/rpc/servers/evented_runner.rb | 43 +++++++++----------- lib/protobuf/rpc/servers/socket_runner.rb | 18 ++++---- lib/protobuf/rpc/servers/socket_server.rb | 11 ----- protobuf.gemspec | 7 ++-- spec/unit/rpc/servers/evented_server_spec.rb | 34 ++++++---------- spec/unit/rpc/servers/socket_server_spec.rb | 8 +++- 8 files changed, 57 insertions(+), 70 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 1e917f79..991a8dfa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -3,12 +3,15 @@ PATH specs: protobuf (1.0.1) eventmachine (~> 0.12.10) + json_pure GEM remote: http://rubygems.org/ specs: - diff-lcs (1.1.2) + diff-lcs (1.1.3) eventmachine (0.12.10) + eventmachine (0.12.10-java) + json_pure (1.6.3) rake (0.8.7) rspec (2.7.0) rspec-core (~> 2.7.0) diff --git a/lib/protobuf.rb b/lib/protobuf.rb index 91d4c9bd..01ce3c68 100644 --- a/lib/protobuf.rb +++ b/lib/protobuf.rb @@ -16,6 +16,7 @@ module Protobuf # For running the rpc_server unless defined?(Protobuf::ServerType) && Protobuf::ServerType == "SocketServer" + Protobuf::ServerType = "EventedServer" require 'eventmachine' require 'protobuf/ext/eventmachine' require 'protobuf/rpc/servers/evented_server' diff --git a/lib/protobuf/rpc/servers/evented_runner.rb b/lib/protobuf/rpc/servers/evented_runner.rb index e72b0878..951f09c1 100644 --- a/lib/protobuf/rpc/servers/evented_runner.rb +++ b/lib/protobuf/rpc/servers/evented_runner.rb @@ -2,33 +2,30 @@ module Protobuf module Rpc class EventedRunner - class << self - - def stop - EventMachine.stop_event_loop if EventMachine.reactor_running? - Protobuf::Logger.info 'Shutdown complete' - end - - def run(server) - # Ensure errors thrown within EM are caught and logged appropriately - EventMachine.error_handler do |error| - if error.message == 'no acceptor' - raise 'Failed binding to %s:%d (%s)' % [server.host, server.port, error.message] - else - Protobuf::Logger.error error.message - Protobuf::Logger.error error.backtrace.join("\n") - end - end + def self.stop + EventMachine.stop_event_loop if EventMachine.reactor_running? + Protobuf::Logger.info 'Shutdown complete' + end - # Startup and run the rpc server - EM.schedule do - EventMachine.start_server(server.host, server.port, Protobuf::Rpc::EventedServer) && \ - Protobuf::Logger.info('RPC Server listening at %s:%d in %s' % [server.host, server.port, server.env]) + def self.run(server) + # Ensure errors thrown within EM are caught and logged appropriately + EventMachine.error_handler do |error| + if error.message == 'no acceptor' + raise 'Failed binding to %s:%d (%s)' % [server.host, server.port, error.message] + else + Protobuf::Logger.error error.message + Protobuf::Logger.error error.backtrace.join("\n") end + end - # Join or start the reactor - EM.reactor_running? ? EM.reactor_thread.join : EM.run + # Startup and run the rpc server + EM.schedule do + EventMachine.start_server(server.host, server.port, Protobuf::Rpc::EventedServer) && \ + Protobuf::Logger.info('RPC Server listening at %s:%d in %s' % [server.host, server.port, server.env]) end + + # Join or start the reactor + EM.reactor_running? ? EM.reactor_thread.join : EM.run end end end diff --git a/lib/protobuf/rpc/servers/socket_runner.rb b/lib/protobuf/rpc/servers/socket_runner.rb index 81f4f846..e0249f24 100644 --- a/lib/protobuf/rpc/servers/socket_runner.rb +++ b/lib/protobuf/rpc/servers/socket_runner.rb @@ -2,18 +2,16 @@ module Protobuf module Rpc class SocketRunner - class << self - def stop - Protobuf::Rpc::SocketServer.stop - Protobuf::Logger.info 'Shutdown complete' - end - - def run(server) - Protobuf::Logger.info "SocketServer Running" - Protobuf::Rpc::SocketServer.run(server.host, server.port) if !Protobuf::Rpc::SocketServer.running? - end + def self.stop + Protobuf::Rpc::SocketServer.stop + Protobuf::Logger.info 'Shutdown complete' end + def self.run(server) + Protobuf::Logger.info "SocketServer Running" + Protobuf::Rpc::SocketServer.run(server.host, server.port) if !Protobuf::Rpc::SocketServer.running? + end end + end end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index 1999020e..791d929f 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -85,17 +85,6 @@ def run(host = "127.0.0.1", port = 9399, backlog = 100, thread_threshold = 100) end end -# # Threaded server -# while true -# @threads << Thread.new(@server.accept) do |sock| -# log_debug "[#{log_signature}] Accepted new connection" -# Protobuf::Rpc::SocketServer::Worker.new(sock) -# sock.close -# end -# -# cleanup_threads if cleanup? -# end - rescue # Closing the server causes the loop to raise an exception here raise if running? diff --git a/protobuf.gemspec b/protobuf.gemspec index 9b805b4a..7530d36d 100644 --- a/protobuf.gemspec +++ b/protobuf.gemspec @@ -1,4 +1,4 @@ -# -*- encoding: utf-8 -*- +# encoding: UTF-8 $:.push File.expand_path("./lib", File.dirname(__FILE__)) require "protobuf/version" @@ -8,7 +8,7 @@ Gem::Specification.new do |s| s.date = %q{2011-12-07} s.authors = ['BJ Neilsen', 'Brandon Dewitt'] - s.email = ["bj.neilsen@gmail.com", "brandonsdewitt@gmail.com"] + s.email = ["bj.neilsen@gmail.com", "brandonsdewitt+protobuf@gmail.com"] s.homepage = %q{https://github.com/localshred/protobuf} s.summary = 'Ruby implementation for Protocol Buffers. Works with other protobuf rpc implementations (e.g. Java, Python, C++).' s.description = s.summary + "\n\nThis gem has diverged from https://github.com/macks/ruby-protobuf. All credit for serialization and rprotoc work most certainly goes to the original authors. All RPC implementation code (client/server/service) was written and is maintained by this author. Attempts to reconcile the original codebase with the current RPC implementation went unsuccessful." @@ -19,7 +19,8 @@ Gem::Specification.new do |s| s.require_paths = ["lib"] s.add_dependency 'eventmachine', '~> 0.12.10' - + s.add_dependency 'json_pure' + s.add_development_dependency 'rake', '~> 0.8.7' s.add_development_dependency 'rspec', '~> 2.7.0' end diff --git a/spec/unit/rpc/servers/evented_server_spec.rb b/spec/unit/rpc/servers/evented_server_spec.rb index fab2c005..90655a20 100644 --- a/spec/unit/rpc/servers/evented_server_spec.rb +++ b/spec/unit/rpc/servers/evented_server_spec.rb @@ -1,26 +1,18 @@ require 'spec_helper' require 'spec/proto/test_service_impl' +require 'protobuf/rpc/servers/evented_runner' -describe Protobuf::Rpc::Server do - context 'when sending response objects' do - it 'should be able to send a hash object as a response' do - server = Protobuf::Rpc::EventedServer.new - - # Setup the right mocks - server.instance_variable_set(:@klass, Spec::Proto::TestService) - response_wrapper = mock('response') - response_wrapper.stub(:response_proto=) - server.instance_variable_set(:@response, response_wrapper) - Spec::Proto::TestService.stub_chain(:rpcs, :[], :[], :response_type).and_return(Spec::Proto::ResourceFindRequest) - - # Setup expectations - hash_response = {:name => 'Test Name', :active => false} - expected = Spec::Proto::ResourceFindRequest.new(hash_response) - Spec::Proto::ResourceFindRequest.should_receive(:new).with(hash_response).and_return(expected) - server.should_not_receive(:handle_error) - - # Call the method - server.send(:parse_response_from_service, hash_response) - end +describe Protobuf::Rpc::EventedServer do + + it "provides a Runner class" do + runner_class = described_class.to_s.gsub(/Server/, "Runner") + expect { Protobuf::Util.constantize(runner_class) }.to_not raise_error + end + + it "Runner provides a stop method" do + runner_class = described_class.to_s.gsub(/Server/, "Runner") + runner_class = Protobuf::Util.constantize(runner_class) + runner_class.respond_to?(:stop).should be_true end + end diff --git a/spec/unit/rpc/servers/socket_server_spec.rb b/spec/unit/rpc/servers/socket_server_spec.rb index 18ce6d72..0f25f077 100644 --- a/spec/unit/rpc/servers/socket_server_spec.rb +++ b/spec/unit/rpc/servers/socket_server_spec.rb @@ -14,6 +14,12 @@ Thread.kill(@server_thread) end + it "Runner provides a stop method" do + runner_class = described_class.to_s.gsub(/Server/, "Runner") + runner_class = Protobuf::Util.constantize(runner_class) + runner_class.respond_to?(:stop).should be_true + end + it "provides a stop method" do described_class.respond_to?(:stop).should be_true end @@ -24,7 +30,7 @@ end it "signals the Server is running" do - Protobuf::Rpc::SocketServer.running?.should be_true + described_class.running?.should be_true end context "Eventmachine client" do From fbace4dc1f18b82000792cdc57939e064ea99efb Mon Sep 17 00:00:00 2001 From: = Date: Wed, 14 Dec 2011 20:54:33 -0700 Subject: [PATCH 47/50] add backlog and threshold to server conf params --- bin/rpc_server | 10 ++++++++++ lib/protobuf/rpc/servers/socket_runner.rb | 2 +- lib/protobuf/rpc/servers/socket_server.rb | 5 +---- spec/unit/rpc/servers/socket_server_spec.rb | 2 +- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/bin/rpc_server b/bin/rpc_server index 3a6f65c3..cf516f84 100755 --- a/bin/rpc_server +++ b/bin/rpc_server @@ -13,6 +13,8 @@ server = OpenStruct.new({ :env => ENV['RPC_SERVER_ENV'] || 'development', :host => '127.0.0.1', :port => 9595, + :backlog => 100, + :threshold => 100, :log => File.expand_path('./protobuf.log'), :level => ::Logger::INFO, :runner => Protobuf::Rpc::EventedRunner, @@ -42,6 +44,14 @@ parser = OptionParser.new do |opts| server.level = v.to_i end + opts.on("-b", "--backlog=N", Integer, "Backlog for listening socket when using Socket Server") do |v| + server.backlog = v.to_i + end + + opts.on("-tt", "--threshold=N", Integer, "Multi-threaded Socket Server threshold for cleanup") do |v| + server.threshold = v.to_i + end + opts.on("-cs", "--client_socket", "Socket Mode for client connections (No EventMachine)") do |v| Protobuf::ConnectorType = "Socket" end diff --git a/lib/protobuf/rpc/servers/socket_runner.rb b/lib/protobuf/rpc/servers/socket_runner.rb index e0249f24..876724df 100644 --- a/lib/protobuf/rpc/servers/socket_runner.rb +++ b/lib/protobuf/rpc/servers/socket_runner.rb @@ -9,7 +9,7 @@ def self.stop def self.run(server) Protobuf::Logger.info "SocketServer Running" - Protobuf::Rpc::SocketServer.run(server.host, server.port) if !Protobuf::Rpc::SocketServer.running? + Protobuf::Rpc::SocketServer.run(server.host, server.port, server.backlog, server.threshold) if !Protobuf::Rpc::SocketServer.running? end end diff --git a/lib/protobuf/rpc/servers/socket_server.rb b/lib/protobuf/rpc/servers/socket_server.rb index 791d929f..0486cdd2 100644 --- a/lib/protobuf/rpc/servers/socket_server.rb +++ b/lib/protobuf/rpc/servers/socket_server.rb @@ -33,7 +33,6 @@ def log_signature @log_signature ||= "server-#{self}" end - # TODO fiber way to get a new worker? def new_worker(socket) Thread.new(socket) do |sock| Protobuf::Rpc::SocketServer::Worker.new(sock) do |s| @@ -65,12 +64,10 @@ def run(host = "127.0.0.1", port = 9399, backlog = 100, thread_threshold = 100) when client == @server then log_debug "[#{log_signature}] Accepted new connection" client, sockaddr = @server.accept - log_debug "--------------->#{sockaddr}" @listen_fds << client else if !@working.include?(client) - @listen_fds.delete(client) - @working << client + @working << @listen_fds.delete(client) log_debug "[#{log_signature}] Working" @threads << { :thread => new_worker(client), :socket => client } diff --git a/spec/unit/rpc/servers/socket_server_spec.rb b/spec/unit/rpc/servers/socket_server_spec.rb index 0f25f077..1542b7f1 100644 --- a/spec/unit/rpc/servers/socket_server_spec.rb +++ b/spec/unit/rpc/servers/socket_server_spec.rb @@ -4,7 +4,7 @@ describe Protobuf::Rpc::SocketServer do before(:all) do - server = OpenStruct.new(:server => "127.0.0.1", :port => 9399) + server = OpenStruct.new(:server => "127.0.0.1", :port => 9399, :backlog => 100, :threshold => 100) @server_thread = Thread.new(server) { |s| Protobuf::Rpc::SocketRunner.run(s) } Thread.pass until Protobuf::Rpc::SocketServer.running? end From fc34269d0d0340bb2613db61f082d27690ded942 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 14 Dec 2011 21:03:14 -0700 Subject: [PATCH 48/50] rpc_server to print help summary when no params provided --- bin/rpc_server | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bin/rpc_server b/bin/rpc_server index cf516f84..5357835f 100755 --- a/bin/rpc_server +++ b/bin/rpc_server @@ -48,7 +48,7 @@ parser = OptionParser.new do |opts| server.backlog = v.to_i end - opts.on("-tt", "--threshold=N", Integer, "Multi-threaded Socket Server threshold for cleanup") do |v| + opts.on("-tt", "--threshold=N", Integer, "Multi-threaded Socket Server cleanup threshold") do |v| server.threshold = v.to_i end @@ -91,7 +91,9 @@ end begin if ARGV.empty? - raise 'You must specify an app file to use.' + puts 'You must specify an app file to use.' + puts parser.help + exit else server.app = ARGV.pop raise 'Invalid app file specified (%s).' % server.app unless File.exists?(server.app) From cd722e8b3920a3ec1fbc04c1ecfdd355c0c2cb0d Mon Sep 17 00:00:00 2001 From: = Date: Wed, 14 Dec 2011 22:25:55 -0700 Subject: [PATCH 49/50] rework backlog and tt --- bin/rpc_server | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/rpc_server b/bin/rpc_server index 5357835f..1accf8ba 100755 --- a/bin/rpc_server +++ b/bin/rpc_server @@ -44,15 +44,15 @@ parser = OptionParser.new do |opts| server.level = v.to_i end - opts.on("-b", "--backlog=N", Integer, "Backlog for listening socket when using Socket Server") do |v| + opts.on("-b N", "--backlog=N", Integer, "Backlog for listening socket when using Socket Server") do |v| server.backlog = v.to_i end - opts.on("-tt", "--threshold=N", Integer, "Multi-threaded Socket Server cleanup threshold") do |v| + opts.on("-t N", "--threshold=N", Integer, "Multi-threaded Socket Server cleanup threshold") do |v| server.threshold = v.to_i end - opts.on("-cs", "--client_socket", "Socket Mode for client connections (No EventMachine)") do |v| + opts.on("-c", "--client_socket", "Socket Mode for client connections (No EventMachine)") do |v| Protobuf::ConnectorType = "Socket" end From 9c15f47c7e90e5cf05f4fa2a6b12a844c6a469f6 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 14 Dec 2011 23:31:03 -0700 Subject: [PATCH 50/50] fix typo in rpc service class --- lib/protobuf/rpc/service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/protobuf/rpc/service.rb b/lib/protobuf/rpc/service.rb index 05d2d920..d31c0e0b 100644 --- a/lib/protobuf/rpc/service.rb +++ b/lib/protobuf/rpc/service.rb @@ -181,7 +181,7 @@ def send_response private def log_and_raise_error(error_message) - logg_error(error_message) + log_error(error_message) raise error_message end