You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by jf...@apache.org on 2013/03/05 03:00:51 UTC

git commit: Thrift-1629:Ruby 1.9 Compatibility during Thrift configure, make, install Client: Ruby Patch: Nick Zalabak

Updated Branches:
  refs/heads/master 04f83112f -> 073f9eb9b


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.


Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/073f9eb9
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/073f9eb9
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/073f9eb9

Branch: refs/heads/master
Commit: 073f9eb9b920bda948b306ee16e78743e42b7cd1
Parents: 04f8311
Author: Jake Farrell <jf...@apache.org>
Authored: Mon Mar 4 20:59:38 2013 -0500
Committer: Jake Farrell <jf...@apache.org>
Committed: Mon Mar 4 20:59:38 2013 -0500

----------------------------------------------------------------------
 lib/rb/lib/thrift.rb                            |    1 +
 lib/rb/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(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/thrift/blob/073f9eb9/lib/rb/lib/thrift.rb
----------------------------------------------------------------------
diff --git a/lib/rb/lib/thrift.rb b/lib/rb/lib/thrift.rb
index fb9e04a..2443ebd 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'

http://git-wip-us.apache.org/repos/asf/thrift/blob/073f9eb9/lib/rb/lib/thrift/server/mongrel_http_server.rb
----------------------------------------------------------------------
diff --git a/lib/rb/lib/thrift/server/mongrel_http_server.rb b/lib/rb/lib/thrift/server/mongrel_http_server.rb
index 84eacf0..de354c8 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
+# <b>DEPRECATED:</b> Please use <tt>Thrift::ThinHTTPServer</tt> 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] || ""

http://git-wip-us.apache.org/repos/asf/thrift/blob/073f9eb9/lib/rb/lib/thrift/server/thin_http_server.rb
----------------------------------------------------------------------
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 0000000..4a81c6d
--- /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

http://git-wip-us.apache.org/repos/asf/thrift/blob/073f9eb9/lib/rb/spec/mongrel_http_server_spec.rb
----------------------------------------------------------------------
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 fa11b35..0000000
--- 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

http://git-wip-us.apache.org/repos/asf/thrift/blob/073f9eb9/lib/rb/spec/thin_http_server_spec.rb
----------------------------------------------------------------------
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 0000000..f17ea92
--- /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
+

http://git-wip-us.apache.org/repos/asf/thrift/blob/073f9eb9/lib/rb/thrift.gemspec
----------------------------------------------------------------------
diff --git a/lib/rb/thrift.gemspec b/lib/rb/thrift.gemspec
index 299db08..67cc3c9 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