svn commit: r666463 - in /incubator/thrift/trunk/lib/alterl/src: thrift_binary_protocol.erl thrift_client.erl thrift_socket_transport.erl

[dr...@apache.org]

Author: dreiss
Date: Tue Jun 10 18:11:57 2008
New Revision: 666463

URL: http://svn.apache.org/viewvc?rev=666463&view=rev
Log:
Factory-ize generation of thrift_binary_protocol to clean things up a bit and decouple thrift_socket_transport's factory from binary protocol

Test plan: tutorial still runs. Someone who actually uses the Options to thrift_client should test this.

Modified:
    incubator/thrift/trunk/lib/alterl/src/thrift_binary_protocol.erl
    incubator/thrift/trunk/lib/alterl/src/thrift_client.erl
    incubator/thrift/trunk/lib/alterl/src/thrift_socket_transport.erl

Modified: incubator/thrift/trunk/lib/alterl/src/thrift_binary_protocol.erl
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/alterl/src/thrift_binary_protocol.erl?rev=666463&r1=666462&r2=666463&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/alterl/src/thrift_binary_protocol.erl (original)
+++ incubator/thrift/trunk/lib/alterl/src/thrift_binary_protocol.erl Tue Jun 10 18:11:57 2008
@@ -15,7 +15,9 @@
          read/2,
          write/2,
          flush_transport/1,
-         close_transport/1
+         close_transport/1,
+
+         new_protocol_factory/2
         ]).
 
 -record(binary_protocol, {transport,
@@ -271,3 +273,30 @@
 read(This, 0) -> {ok, <<>>};
 read(This, Len) when is_integer(Len), Len >= 0 ->
     thrift_transport:read(This#binary_protocol.transport, Len).
+
+
+%%%% FACTORY GENERATION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+
+-record(tbp_opts, {strict_read = true,
+                   strict_write = true}).
+
+parse_factory_options([], Opts) ->
+    Opts;
+parse_factory_options([{strict_read, Bool} | Rest], Opts) when is_boolean(Bool) ->
+    parse_factory_options(Rest, Opts#tbp_opts{strict_read=Bool});
+parse_factory_options([{strict_write, Bool} | Rest], Opts) when is_boolean(Bool) ->
+    parse_factory_options(Rest, Opts#tbp_opts{strict_write=Bool}).
+
+
+%% returns a (fun() -> thrift_protocol())
+new_protocol_factory(TransportFactory, Options) ->
+    ParsedOpts = parse_factory_options(Options, #tbp_opts{}),
+    F = fun() ->
+                {ok, Transport} = TransportFactory(),
+                thrift_binary_protocol:new(
+                  Transport,
+                  [{strict_read,  ParsedOpts#tbp_opts.strict_read},
+                   {strict_write, ParsedOpts#tbp_opts.strict_write}])
+        end,
+    {ok, F}.
+    

Modified: incubator/thrift/trunk/lib/alterl/src/thrift_client.erl
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/alterl/src/thrift_client.erl?rev=666463&r1=666462&r2=666463&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/alterl/src/thrift_client.erl (original)
+++ incubator/thrift/trunk/lib/alterl/src/thrift_client.erl Tue Jun 10 18:11:57 2008
@@ -32,12 +32,44 @@
 start_link(Host, Port, Service) when is_integer(Port), is_atom(Service) ->
     start_link(Host, Port, Service, []).
 
-%% Backwards-compatible starter for the usual case of socket transports
+
+%%
+%% Splits client options into protocol options and transport options
+%%
+%% split_options([Options...]) -> {ProtocolOptions, TransportOptions}
+%%
+split_options(Options) ->
+    split_options(Options, [], []).
+
+split_options([], ProtoIn, TransIn) ->
+    {ProtoIn, TransIn};
+
+split_options([Opt = {OptKey, _} | Rest], ProtoIn, TransIn)
+  when OptKey =:= strict_read;
+       OptKey =:= strict_write ->
+    split_options(Rest, [Opt | ProtoIn], TransIn);
+
+split_options([Opt = {OptKey, _} | Rest], ProtoIn, TransIn)
+  when OptKey =:= framed;
+       OptKey =:= connect_timeout;
+       OptKey =:= sockopts ->
+    split_options(Rest, ProtoIn, [Opt | TransIn]).
+
+
+%% Backwards-compatible starter for the common-case of socket transports
 start_link(Host, Port, Service, Options)
   when is_integer(Port), is_atom(Service), is_list(Options) ->
-    {ok, ProtocolFactory} = thrift_socket_transport:new_protocol_factory(Host, Port, Options),
+    {ProtoOpts, TransOpts} = split_options(Options),
+
+    {ok, TransportFactory} =
+        thrift_socket_transport:new_transport_factory(Host, Port, TransOpts),
+
+    {ok, ProtocolFactory} = thrift_binary_protocol:new_protocol_factory(
+                              TransportFactory, ProtoOpts),
+
     start_link(ProtocolFactory, Service).
 
+
 %% ProtocolFactory :: fun() -> thrift_protocol()
 start_link(ProtocolFactory, Service)
   when is_function(ProtocolFactory), is_atom(Service) ->

Modified: incubator/thrift/trunk/lib/alterl/src/thrift_socket_transport.erl
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/alterl/src/thrift_socket_transport.erl?rev=666463&r1=666462&r2=666463&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/alterl/src/thrift_socket_transport.erl (original)
+++ incubator/thrift/trunk/lib/alterl/src/thrift_socket_transport.erl Tue Jun 10 18:11:57 2008
@@ -6,7 +6,7 @@
          new/2,
          write/2, read/2, flush/1, close/1,
 
-         new_protocol_factory/3]).
+         new_transport_factory/3]).
 
 -record(data, {socket,
                recv_timeout=infinity}).
@@ -56,16 +56,10 @@
 %% operation instead of O(n^2)
 -record(factory_opts, {connect_timeout = infinity,
                        sockopts = [],
-                       framed = false,
-                       strict_read = true,
-                       strict_write = true}).
+                       framed = false}).
 
 parse_factory_options([], Opts) ->
     Opts;
-parse_factory_options([{strict_read, Bool} | Rest], Opts) when is_boolean(Bool) ->
-    parse_factory_options(Rest, Opts#factory_opts{strict_read=Bool});
-parse_factory_options([{strict_write, Bool} | Rest], Opts) when is_boolean(Bool) ->
-    parse_factory_options(Rest, Opts#factory_opts{strict_write=Bool});
 parse_factory_options([{framed, Bool} | Rest], Opts) when is_boolean(Bool) ->
     parse_factory_options(Rest, Opts#factory_opts{framed=Bool});
 parse_factory_options([{sockopts, OptList} | Rest], Opts) when is_list(OptList) ->
@@ -73,12 +67,14 @@
 parse_factory_options([{connect_timeout, TO} | Rest], Opts) when TO =:= infinity; is_integer(TO) ->
     parse_factory_options(Rest, Opts#factory_opts{connect_timeout=TO}).
 
+
 %%
-%% Generates a "protocol factory" function - a fun which returns a Protocol instance.
-%% This can be passed to thrift_client:start_link in order to connect to a
-%% server over a socket.
+%% Generates a "transport factory" function - a fun which returns a thrift_transport()
+%% instance.
+%% This can be passed into a protocol factory to generate a connection to a
+%% thrift server over a socket.
 %%
-new_protocol_factory(Host, Port, Options) ->
+new_transport_factory(Host, Port, Options) ->
     ParsedOpts = parse_factory_options(Options, #factory_opts{}),
 
     F = fun() ->
@@ -96,10 +92,7 @@
                                 true  -> thrift_framed_transport:new(Transport);
                                 false -> thrift_buffered_transport:new(Transport)
                             end,
-                        thrift_binary_protocol:new(
-                          BufTransport,
-                          [{strict_read,  ParsedOpts#factory_opts.strict_read},
-                           {strict_write, ParsedOpts#factory_opts.strict_write}]);
+                        {ok, BufTransport};
                     Error  ->
                         Error
                 end