From 073f9eb9b920bda948b306ee16e78743e42b7cd1 Mon Sep 17 00:00:00 2001 From: Jake Farrell Date: Mon, 4 Mar 2013 20:59:38 -0500 Subject: [PATCH] Thrift-1629:Ruby 1.9 Compatibility during Thrift configure, make, install Client: Ruby Patch: Nick Zalabak Updated ruby client to use thin serber over mongrel. --- lib/rb/lib/thrift.rb | 1 + .../lib/thrift/server/mongrel_http_server.rb | 2 + lib/rb/lib/thrift/server/thin_http_server.rb | 91 ++++++++++++ lib/rb/spec/mongrel_http_server_spec.rb | 114 -------------- lib/rb/spec/thin_http_server_spec.rb | 140 ++++++++++++++++++ lib/rb/thrift.gemspec | 7 +- 6 files changed, 239 insertions(+), 116 deletions(-) create mode 100644 lib/rb/lib/thrift/server/thin_http_server.rb delete mode 100644 lib/rb/spec/mongrel_http_server_spec.rb create mode 100644 lib/rb/spec/thin_http_server_spec.rb diff --git a/lib/rb/lib/thrift.rb b/lib/rb/lib/thrift.rb index fb9e04a2..2443ebd1 100644 --- a/lib/rb/lib/thrift.rb +++ b/lib/rb/lib/thrift.rb @@ -62,5 +62,6 @@ require 'thrift/server/nonblocking_server' require 'thrift/server/simple_server' require 'thrift/server/threaded_server' require 'thrift/server/thread_pool_server' +require 'thrift/server/thin_http_server' require 'thrift/thrift_native' diff --git a/lib/rb/lib/thrift/server/mongrel_http_server.rb b/lib/rb/lib/thrift/server/mongrel_http_server.rb index 84eacf0d..de354c8f 100644 --- a/lib/rb/lib/thrift/server/mongrel_http_server.rb +++ b/lib/rb/lib/thrift/server/mongrel_http_server.rb @@ -20,6 +20,7 @@ require 'mongrel' ## Sticks a service on a URL, using mongrel to do the HTTP work +# DEPRECATED: Please use Thrift::ThinHTTPServer instead. module Thrift class MongrelHTTPServer < BaseServer class Handler < Mongrel::HttpHandler @@ -43,6 +44,7 @@ module Thrift end def initialize(processor, opts={}) + Kernel.warn "[DEPRECATION WARNING] `Thrift::MongrelHTTPServer` is deprecated. Please use `Thrift::ThinHTTPServer` instead." port = opts[:port] || 80 ip = opts[:ip] || "0.0.0.0" path = opts[:path] || "" diff --git a/lib/rb/lib/thrift/server/thin_http_server.rb b/lib/rb/lib/thrift/server/thin_http_server.rb new file mode 100644 index 00000000..4a81c6d1 --- /dev/null +++ b/lib/rb/lib/thrift/server/thin_http_server.rb @@ -0,0 +1,91 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +require 'rack' +require 'thin' + +## +# Wraps the Thin web server to provide a Thrift server over HTTP. +module Thrift + class ThinHTTPServer < BaseServer + + ## + # Accepts a Thrift::Processor + # Options include: + # * :port + # * :ip + # * :path + # * :protocol_factory + def initialize(processor, options={}) + port = options[:port] || 80 + ip = options[:ip] || "0.0.0.0" + path = options[:path] || "/" + protocol_factory = options[:protocol_factory] || BinaryProtocolFactory.new + app = RackApplication.for(path, processor, protocol_factory) + @server = Thin::Server.new(ip, port, app) + end + + ## + # Starts the server + def serve + @server.start + end + + class RackApplication + + THRIFT_HEADER = "application/x-thrift" + + def self.for(path, processor, protocol_factory) + Rack::Builder.new do + use Rack::CommonLogger + use Rack::ShowExceptions + use Rack::Lint + map path do + run lambda { |env| + request = Rack::Request.new(env) + if RackApplication.valid_thrift_request?(request) + RackApplication.successful_request(request, processor, protocol_factory) + else + RackApplication.failed_request + end + } + end + end + end + + def self.successful_request(rack_request, processor, protocol_factory) + response = Rack::Response.new([], 200, {'Content-Type' => THRIFT_HEADER}) + transport = IOStreamTransport.new rack_request.body, response + protocol = protocol_factory.get_protocol transport + processor.process protocol, protocol + response + end + + def self.failed_request + Rack::Response.new(['Not Found'], 404, {'Content-Type' => THRIFT_HEADER}) + end + + def self.valid_thrift_request?(rack_request) + rack_request.post? && rack_request.env["CONTENT_TYPE"] == THRIFT_HEADER + end + + end + + end +end diff --git a/lib/rb/spec/mongrel_http_server_spec.rb b/lib/rb/spec/mongrel_http_server_spec.rb deleted file mode 100644 index fa11b35f..00000000 --- a/lib/rb/spec/mongrel_http_server_spec.rb +++ /dev/null @@ -1,114 +0,0 @@ -# -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# - -require 'spec_helper' -require 'thrift/server/mongrel_http_server' - -describe 'HTTPServer' do - - describe Thrift::MongrelHTTPServer do - it "should have appropriate defaults" do - mock_factory = mock("BinaryProtocolFactory") - mock_proc = mock("Processor") - Thrift::BinaryProtocolFactory.should_receive(:new).and_return(mock_factory) - Mongrel::HttpServer.should_receive(:new).with("0.0.0.0", 80).and_return do - mock("Mongrel::HttpServer").tap do |mock| - handler = mock("Handler") - Thrift::MongrelHTTPServer::Handler.should_receive(:new).with(mock_proc, mock_factory).and_return(handler) - mock.should_receive(:register).with("/", handler) - end - end - Thrift::MongrelHTTPServer.new(mock_proc) - end - - it "should understand :ip, :port, :path, and :protocol_factory" do - mock_proc = mock("Processor") - mock_factory = mock("ProtocolFactory") - Mongrel::HttpServer.should_receive(:new).with("1.2.3.4", 1234).and_return do - mock("Mongrel::HttpServer").tap do |mock| - handler = mock("Handler") - Thrift::MongrelHTTPServer::Handler.should_receive(:new).with(mock_proc, mock_factory).and_return(handler) - mock.should_receive(:register).with("/foo", handler) - end - end - Thrift::MongrelHTTPServer.new(mock_proc, :ip => "1.2.3.4", :port => 1234, :path => "foo", - :protocol_factory => mock_factory) - end - - it "should serve using Mongrel::HttpServer" do - Thrift::BinaryProtocolFactory.stub!(:new) - Mongrel::HttpServer.should_receive(:new).and_return do - mock("Mongrel::HttpServer").tap do |mock| - Thrift::MongrelHTTPServer::Handler.stub!(:new) - mock.stub!(:register) - mock.should_receive(:run).and_return do - mock("Mongrel::HttpServer.run").tap do |runner| - runner.should_receive(:join) - end - end - end - end - Thrift::MongrelHTTPServer.new(nil).serve - end - end - - describe Thrift::MongrelHTTPServer::Handler do - before(:each) do - @processor = mock("Processor") - @factory = mock("ProtocolFactory") - @handler = described_class.new(@processor, @factory) - end - - it "should return 404 for non-POST requests" do - request = mock("request", :params => {"REQUEST_METHOD" => "GET"}) - response = mock("response") - response.should_receive(:start).with(404) - response.should_not_receive(:start).with(200) - @handler.process(request, response) - end - - it "should serve using application/x-thrift" do - request = mock("request", :params => {"REQUEST_METHOD" => "POST"}, :body => nil) - response = mock("response") - head = mock("head") - head.should_receive(:[]=).with("Content-Type", "application/x-thrift") - Thrift::IOStreamTransport.stub!(:new) - @factory.stub!(:get_protocol) - @processor.stub!(:process) - response.should_receive(:start).with(200).and_yield(head, nil) - @handler.process(request, response) - end - - it "should use the IOStreamTransport" do - body = mock("body") - request = mock("request", :params => {"REQUEST_METHOD" => "POST"}, :body => body) - response = mock("response") - head = mock("head") - head.stub!(:[]=) - out = mock("out") - protocol = mock("protocol") - transport = mock("transport") - Thrift::IOStreamTransport.should_receive(:new).with(body, out).and_return(transport) - @factory.should_receive(:get_protocol).with(transport).and_return(protocol) - @processor.should_receive(:process).with(protocol, protocol) - response.should_receive(:start).with(200).and_yield(head, out) - @handler.process(request, response) - end - end -end diff --git a/lib/rb/spec/thin_http_server_spec.rb b/lib/rb/spec/thin_http_server_spec.rb new file mode 100644 index 00000000..f17ea924 --- /dev/null +++ b/lib/rb/spec/thin_http_server_spec.rb @@ -0,0 +1,140 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +require 'spec_helper' +require 'rack/test' + +describe Thrift::ThinHTTPServer do + + let(:processor) { mock('processor') } + + describe "#initialize" do + + context "when using the defaults" do + + it "binds to port 80, with host 0.0.0.0, a path of '/'" do + Thin::Server.should_receive(:new).with('0.0.0.0', 80, an_instance_of(Rack::Builder)) + Thrift::ThinHTTPServer.new(processor) + end + + it 'creates a ThinHTTPServer::RackApplicationContext' do + Thrift::ThinHTTPServer::RackApplication.should_receive(:for).with("/", processor, an_instance_of(Thrift::BinaryProtocolFactory)).and_return(anything) + Thrift::ThinHTTPServer.new(processor) + end + + it "uses the BinaryProtocolFactory" do + Thrift::BinaryProtocolFactory.should_receive(:new) + Thrift::ThinHTTPServer.new(processor) + end + + end + + context "when using the options" do + + it 'accepts :ip, :port, :path' do + ip = "192.168.0.1" + port = 3000 + path = "/thin" + Thin::Server.should_receive(:new).with(ip, port, an_instance_of(Rack::Builder)) + Thrift::ThinHTTPServer.new(processor, + :ip => ip, + :port => port, + :path => path) + end + + it 'creates a ThinHTTPServer::RackApplicationContext with a different protocol factory' do + Thrift::ThinHTTPServer::RackApplication.should_receive(:for).with("/", processor, an_instance_of(Thrift::JsonProtocolFactory)).and_return(anything) + Thrift::ThinHTTPServer.new(processor, + :protocol_factory => Thrift::JsonProtocolFactory.new) + end + + end + + end + + describe "#serve" do + + it 'starts the Thin server' do + underlying_thin_server = mock('thin server', :start => true) + Thin::Server.stub(:new).and_return(underlying_thin_server) + + thin_thrift_server = Thrift::ThinHTTPServer.new(processor) + + underlying_thin_server.should_receive(:start) + thin_thrift_server.serve + end + end + +end + +describe Thrift::ThinHTTPServer::RackApplication do + include Rack::Test::Methods + + let(:processor) { mock('processor') } + let(:protocol_factory) { mock('protocol factory') } + + def app + Thrift::ThinHTTPServer::RackApplication.for("/", processor, protocol_factory) + end + + context "404 response" do + + it 'receives a non-POST' do + header('Content-Type', "application/x-thrift") + get "/" + last_response.status.should be 404 + end + + it 'receives a header other than application/x-thrift' do + header('Content-Type', "application/json") + post "/" + last_response.status.should be 404 + end + + end + + context "200 response" do + + before do + protocol_factory.stub(:get_protocol) + processor.stub(:process) + end + + it 'creates an IOStreamTransport' do + header('Content-Type', "application/x-thrift") + Thrift::IOStreamTransport.should_receive(:new).with(an_instance_of(Rack::Lint::InputWrapper), an_instance_of(Rack::Response)) + post "/" + end + + it 'fetches the right protocol based on the Transport' do + header('Content-Type', "application/x-thrift") + protocol_factory.should_receive(:get_protocol).with(an_instance_of(Thrift::IOStreamTransport)) + post "/" + end + + it 'status code 200' do + header('Content-Type', "application/x-thrift") + post "/" + last_response.ok?.should be_true + end + + end + +end + diff --git a/lib/rb/thrift.gemspec b/lib/rb/thrift.gemspec index 299db08a..67cc3c9c 100644 --- a/lib/rb/thrift.gemspec +++ b/lib/rb/thrift.gemspec @@ -27,8 +27,11 @@ Gem::Specification.new do |s| s.require_paths = %w[lib ext] - s.add_development_dependency 'rake' s.add_development_dependency 'rspec', '~> 2.10.0' - s.add_development_dependency 'mongrel', "1.2.0.pre2" + s.add_development_dependency "rack", "~> 1.5.2" + s.add_development_dependency "rack-test", "~> 0.6.2" + s.add_development_dependency "thin", "~> 1.5.0" + s.add_development_dependency "bundler", "~> 1.3.1" + s.add_development_dependency 'rake' end -- 2.17.1