THRIFT-599. erl: Don't use unnecessary processes in the Erlang transports and clients

The only user-visible changes are to the client. Every thrift call now returns {NewClient, Result} instead of just Result.

Patch: David Reiss (assist to Anthony Molinaro)

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@987018 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/test/erl/Makefile b/test/erl/Makefile
index 2126037..a6b5ae6 100644
--- a/test/erl/Makefile
+++ b/test/erl/Makefile
@@ -29,7 +29,7 @@
 ALL_INCLUDEDIR=$(GEN_INCLUDEDIR) $(INCLUDEDIR) ../../lib/erl/include
 INCLUDEFLAGS=$(patsubst %,-I%, ${ALL_INCLUDEDIR})
 
-MODULES = stress_server test_server test_disklog test_membuffer test_tether
+MODULES = stress_server test_server test_client test_disklog test_membuffer
 
 INCLUDES = 
 TARGETS = $(patsubst %,${TARGETDIR}/%.beam,${MODULES})
@@ -55,11 +55,11 @@
 ${GEN_TARGETDIR}/: ${GENDIR}/
 	rm -rf ${GEN_TARGETDIR}
 	mkdir -p ${GEN_TARGETDIR}
-	erlc ${INCLUDEFLAGS} -o ${GEN_TARGETDIR} ${GEN_SRCDIR}/*.erl
+	erlc ${ERLC_FLAGS} ${INCLUDEFLAGS} -o ${GEN_TARGETDIR} ${GEN_SRCDIR}/*.erl
 
 $(TARGETS): ${TARGETDIR}/%.beam: ${SRCDIR}/%.erl ${GEN_INCLUDEDIR}/ ${HEADERS}
 	mkdir -p ${TARGETDIR}
-	erlc ${INCLUDEFLAGS} -o ${TARGETDIR} $<
+	erlc ${ERLC_FLAGS} ${INCLUDEFLAGS} -o ${TARGETDIR} $<
 
 clean:
 	rm -f ${TARGETDIR}/*.beam
diff --git a/test/erl/src/test_client.erl b/test/erl/src/test_client.erl
new file mode 100644
index 0000000..a26467f
--- /dev/null
+++ b/test/erl/src/test_client.erl
@@ -0,0 +1,132 @@
+%%
+%% 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.
+%%
+
+-module(test_client).
+
+-export([start/0, start/1]).
+
+-include("thriftTest_types.hrl").
+
+-record(options, {port = 9090,
+                  client_opts = []}).
+
+parse_args(Args) -> parse_args(Args, #options{}).
+parse_args([], Opts) -> Opts;
+parse_args([Head | Rest], Opts) ->
+    NewOpts =
+        case catch list_to_integer(Head) of
+            Port when is_integer(Port) ->
+                Opts#options{port = Port};
+            _Else ->
+                case Head of
+                    "framed" ->
+                        Opts#options{client_opts = [{framed, true} | Opts#options.client_opts]};
+                    "" ->
+                        Opts;
+                    _Else ->
+                        erlang:error({bad_arg, Head})
+                end
+        end,
+    parse_args(Rest, NewOpts).
+
+
+start() -> start([]).
+start(Args) ->
+  #options{port = Port, client_opts = ClientOpts} = parse_args(Args),
+  {ok, Client0} = thrift_client_util:new(
+    "127.0.0.1", Port, thriftTest_thrift, ClientOpts),
+
+  DemoXtruct = #xtruct{
+    string_thing = <<"Zero">>,
+    byte_thing = 1,
+    i32_thing = 9128361,
+    i64_thing = 9223372036854775807},
+
+  DemoNest = #xtruct2{
+    byte_thing = 7,
+    struct_thing = DemoXtruct,
+    % Note that we don't set i32_thing, it will come back as undefined
+    % from the Python server, but 0 from the C++ server, since it is not
+    % optional
+    i32_thing = 2},
+
+  % Is it safe to match these things?
+  DemoDict = dict:from_list([ {Key, Key-10} || Key <- lists:seq(0,10) ]),
+  DemoSet = sets:from_list([ Key || Key <- lists:seq(-3,3) ]),
+
+  %DemoInsane = #insanity{
+  %  userMap = dict:from_list([{?thriftTest_FIVE, 5000}]),
+  %  xtructs = [#xtruct{ string_thing = <<"Truck">>, byte_thing = 8, i32_thing = 8, i64_thing = 8}]},
+
+  {Client01, {ok, ok}} = thrift_client:call(Client0, testVoid, []),
+
+  {Client02, {ok, <<"Test">>}}      = thrift_client:call(Client01, testString, ["Test"]),
+  {Client03, {ok, <<"Test">>}}      = thrift_client:call(Client02, testString, [<<"Test">>]),
+  {Client04, {ok, 63}}              = thrift_client:call(Client03, testByte, [63]),
+  {Client05, {ok, -1}}              = thrift_client:call(Client04, testI32, [-1]),
+  {Client06, {ok, 0}}               = thrift_client:call(Client05, testI32, [0]),
+  {Client07, {ok, -34359738368}}    = thrift_client:call(Client06, testI64, [-34359738368]),
+  {Client08, {ok, -5.2098523}}      = thrift_client:call(Client07, testDouble, [-5.2098523]),
+  {Client09, {ok, DemoXtruct}}      = thrift_client:call(Client08, testStruct, [DemoXtruct]),
+  {Client10, {ok, DemoNest}}        = thrift_client:call(Client09, testNest, [DemoNest]),
+  {Client11, {ok, DemoDict}}        = thrift_client:call(Client10, testMap, [DemoDict]),
+  {Client12, {ok, DemoSet}}         = thrift_client:call(Client11, testSet, [DemoSet]),
+  {Client13, {ok, [-1,2,3]}}        = thrift_client:call(Client12, testList, [[-1,2,3]]),
+  {Client14, {ok, 1}}               = thrift_client:call(Client13, testEnum, [?thriftTest_ONE]),
+  {Client15, {ok, 309858235082523}} = thrift_client:call(Client14, testTypedef, [309858235082523]),
+
+  % No python implementation, but works with C++ and Erlang.
+  %{Client16, {ok, InsaneResult}}    = thrift_client:call(Client15, testInsanity, [DemoInsane]),
+  %io:format("~p~n", [InsaneResult]),
+  Client16 = Client15,
+
+  {Client17, {ok, #xtruct{string_thing = <<"Message">>}}} =
+    thrift_client:call(Client16, testMultiException, ["Safe", "Message"]),
+
+  Client18 =
+    try
+      {ClientS1, Result1} = thrift_client:call(Client17, testMultiException, ["Xception", "Message"]),
+      io:format("Unexpected return! ~p~n", [Result1]),
+      ClientS1
+    catch
+      throw:{ClientS2, {exception, ExnS1 = #xception{}}} ->
+        #xception{errorCode = 1001, message = <<"This is an Xception">>} = ExnS1,
+        ClientS2;
+      throw:{ClientS2, {exception, _ExnS1 = #xception2{}}} ->
+        io:format("Wrong exception type!~n", []),
+        ClientS2
+    end,
+
+  Client19 =
+    try
+      {ClientS3, Result2} = thrift_client:call(Client18, testMultiException, ["Xception2", "Message"]),
+      io:format("Unexpected return! ~p~n", [Result2]),
+      ClientS3
+    catch
+      throw:{ClientS4, {exception, _ExnS2 = #xception{}}} ->
+        io:format("Wrong exception type!~n", []),
+        ClientS4;
+      throw:{ClientS4, {exception, ExnS2 = #xception2{}}} ->
+        #xception2{errorCode = 2002,
+                   struct_thing = #xtruct{
+                     string_thing = <<"This is an Xception2">>}} = ExnS2,
+        ClientS4
+    end,
+
+  thrift_client:close(Client19).
diff --git a/test/erl/src/test_disklog.erl b/test/erl/src/test_disklog.erl
index 7b0be72..fc0dcf8 100644
--- a/test/erl/src/test_disklog.erl
+++ b/test/erl/src/test_disklog.erl
@@ -29,20 +29,21 @@
            {size, {1024*1024, 10}}]),
     {ok, ProtocolFactory} = thrift_binary_protocol:new_protocol_factory(
                               TransportFactory, []),
-    {ok, Client} = thrift_client:start_link(ProtocolFactory, thriftTest_thrift),
+    {ok, Proto} = ProtocolFactory(),
+    {ok, Client0} = thrift_client:new(Proto, thriftTest_thrift),
 
     io:format("Client started~n"),
 
     % We have to make oneway calls into this client only since otherwise it will try
     % to read from the disklog and go boom.
-    {ok, ok} = thrift_client:call(Client, testOneway, [16#deadbeef]),
+    {Client1, {ok, ok}} = thrift_client:call(Client0, testOneway, [16#deadbeef]),
     io:format("Call written~n"),
 
     % Use the send_call method to write a non-oneway call into the log
-    ok = thrift_client:send_call(Client, testString, [<<"hello world">>]),
+    {Client2, ok} = thrift_client:send_call(Client1, testString, [<<"hello world">>]),
     io:format("Non-oneway call sent~n"),
 
-    ok = thrift_client:close(Client),
+    {_Client3, ok} = thrift_client:close(Client2),
     io:format("Client closed~n"),
 
     ok.
@@ -61,21 +62,22 @@
         thrift_buffered_transport:new_transport_factory(B64Factory),
     {ok, ProtocolFactory} = thrift_binary_protocol:new_protocol_factory(
                               BufFactory, []),
-    {ok, Client} = thrift_client:start_link(ProtocolFactory, thriftTest_thrift),
+    {ok, Proto} = ProtocolFactory(),
+    {ok, Client0} = thrift_client:new(Proto, thriftTest_thrift),
 
     io:format("Client started~n"),
 
     % We have to make oneway calls into this client only since otherwise it will try
     % to read from the disklog and go boom.
-    {ok, ok} = thrift_client:call(Client, testOneway, [16#deadbeef]),
+    {Client1, {ok, ok}} = thrift_client:call(Client0, testOneway, [16#deadbeef]),
     io:format("Call written~n"),
 
     % Use the send_call method to write a non-oneway call into the log
-    ok = thrift_client:send_call(Client, testString, [<<"hello world">>]),
+    {Client2, ok} = thrift_client:send_call(Client1, testString, [<<"hello world">>]),
     io:format("Non-oneway call sent~n"),
 
-    ok = thrift_client:close(Client),
+    {_Client3, ok} = thrift_client:close(Client2),
     io:format("Client closed~n"),
 
     ok.
-    
+
diff --git a/test/erl/src/test_membuffer.erl b/test/erl/src/test_membuffer.erl
index 7bd23a0..19ac527 100644
--- a/test/erl/src/test_membuffer.erl
+++ b/test/erl/src/test_membuffer.erl
@@ -30,12 +30,12 @@
 
 t1() ->
     {ok, Transport} = thrift_memory_buffer:new(),
-    {ok, Protocol} = thrift_binary_protocol:new(Transport),
+    {ok, Protocol0} = thrift_binary_protocol:new(Transport),
     TestData = test_data(),
-    ok = thrift_protocol:write(Protocol,
+		{Protocol1, ok} = thrift_protocol:write(Protocol0,
 			       {{struct, element(2, thriftTest_types:struct_info('xtruct'))},
 				TestData}),
-    {ok, Result} = thrift_protocol:read(Protocol,
+		{_Protocol2, {ok, Result}} = thrift_protocol:read(Protocol1,
 					{struct, element(2, thriftTest_types:struct_info('xtruct'))},
 					'xtruct'),
 
@@ -44,12 +44,12 @@
 
 t2() ->
     {ok, Transport} = thrift_memory_buffer:new(),
-    {ok, Protocol} = thrift_binary_protocol:new(Transport),
+    {ok, Protocol0} = thrift_binary_protocol:new(Transport),
     TestData = test_data(),
-    ok = thrift_protocol:write(Protocol,
+		{Protocol1, ok} = thrift_protocol:write(Protocol0,
 			       {{struct, element(2, thriftTest_types:struct_info('xtruct'))},
 				TestData}),
-    {ok, Result} = thrift_protocol:read(Protocol,
+		{_Protocol2, {ok, Result}} = thrift_protocol:read(Protocol1,
 					{struct, element(2, thriftTest_types:struct_info('xtruct3'))},
 					'xtruct3'),
 
@@ -61,12 +61,12 @@
 
 t3() ->
     {ok, Transport} = thrift_memory_buffer:new(),
-    {ok, Protocol} = thrift_binary_protocol:new(Transport),
+    {ok, Protocol0} = thrift_binary_protocol:new(Transport),
     TestData = #bools{im_true = true, im_false = false},
-    ok = thrift_protocol:write(Protocol,
+		{Protocol1, ok} = thrift_protocol:write(Protocol0,
 			       {{struct, element(2, thriftTest_types:struct_info('bools'))},
 				TestData}),
-    {ok, Result} = thrift_protocol:read(Protocol,
+		{_Protocol2, {ok, Result}} = thrift_protocol:read(Protocol1,
 					{struct, element(2, thriftTest_types:struct_info('bools'))},
 					'bools'),
 
@@ -74,8 +74,23 @@
     true = TestData#bools.im_false =:= Result#bools.im_false.
 
 
+t4() ->
+    {ok, Transport} = thrift_memory_buffer:new(),
+    {ok, Protocol0} = thrift_binary_protocol:new(Transport),
+    TestData = #insanity{xtructs=[]},
+		{Protocol1, ok} = thrift_protocol:write(Protocol0,
+			       {{struct, element(2, thriftTest_types:struct_info('insanity'))},
+				TestData}),
+		{_Protocol2, {ok, Result}} = thrift_protocol:read(Protocol1,
+					{struct, element(2, thriftTest_types:struct_info('insanity'))},
+					'insanity'),
+
+    TestData = Result.
+
+
 t() ->
     t1(),
     t2(),
-    t3().
+    t3(),
+    t4().
 
diff --git a/test/erl/src/test_server.erl b/test/erl/src/test_server.erl
index cd439cc..28d47b1 100644
--- a/test/erl/src/test_server.erl
+++ b/test/erl/src/test_server.erl
@@ -19,12 +19,42 @@
 
 -module(test_server).
 
--export([start_link/1, handle_function/2]).
+-export([go/0, go/1, start_link/2, handle_function/2]).
 
 -include("thriftTest_types.hrl").
 
-start_link(Port) ->
-    thrift_server:start_link(Port, thriftTest_thrift, ?MODULE).
+-record(options, {port = 9090,
+                  server_opts = []}).
+
+parse_args(Args) -> parse_args(Args, #options{}).
+parse_args([], Opts) -> Opts;
+parse_args([Head | Rest], Opts) ->
+    NewOpts =
+        case catch list_to_integer(Head) of
+            Port when is_integer(Port) ->
+                Opts#options{port = Port};
+            _Else ->
+                case Head of
+                    "framed" ->
+                        Opts#options{server_opts = [{framed, true} | Opts#options.server_opts]};
+                    "" ->
+                        Opts;
+                    _Else ->
+                        erlang:error({bad_arg, Head})
+                end
+        end,
+    parse_args(Rest, NewOpts).
+
+go() -> go([]).
+go(Args) ->
+    #options{port = Port, server_opts = ServerOpts} = parse_args(Args),
+    spawn(fun() -> start_link(Port, ServerOpts), receive after infinity -> ok end end).
+
+start_link(Port, ServerOpts) ->
+    thrift_socket_server:start([{handler, ?MODULE},
+                                {service, thriftTest_thrift},
+                                {port, Port}] ++
+                               ServerOpts).
 
 
 handle_function(testVoid, {}) ->
@@ -124,12 +154,12 @@
                                {?thriftTest_THREE, Crazy}]),
 
     SecondMap = dict:from_list([{?thriftTest_SIX, Looney}]),
-    
+
     Insane = dict:from_list([{1, FirstMap},
                              {2, SecondMap}]),
-    
+
     io:format("Return = ~p~n", [Insane]),
-    
+
     {reply, Insane};
 
 handle_function(testMulti, Args = {Arg0, Arg1, Arg2, _Arg3, Arg4, Arg5})
@@ -150,7 +180,7 @@
     case String of
         <<"Xception">> ->
             throw(#xception{errorCode = 1001,
-                            message = <<"This is an Xception">>});
+                            message = String});
         _ ->
             ok
     end;
diff --git a/test/erl/src/test_tether.erl b/test/erl/src/test_tether.erl
deleted file mode 100644
index dc11a9a..0000000
--- a/test/erl/src/test_tether.erl
+++ /dev/null
@@ -1,186 +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.
-%%
-%% Tests the behavior of clients in the face of transport errors.
-%% Makes sure start, start_linked, and start_tethered work as expected.
-
--module(test_tether).
-
--compile(export_all).
-
-
-t() ->
-    io:format("Beginning transport error test.~n"),
-    Pid1 = erlang:spawn(?MODULE, t_sub, [2]),
-    wait_for(Pid1),
-    io:format("Beginning protocol error test.~n"),
-    Pid2 = erlang:spawn(?MODULE, t_sub, [22]),
-    wait_for(Pid2),
-    ok.
-
-t_sub(Port) ->
-    io:format("Starting.~n", []),
-    register(tester, self()),
-
-    Pid1 = erlang:spawn(?MODULE, test_start, [Port]),
-    receive after 200 -> ok end,  % Wait for completion.
-    case is_up(Pid1) of
-        true ->
-            io:format("PASS.  Unlinked owner still alive.~n");
-        false ->
-            io:format("FAIL.  Unlinked owner is dead.~n")
-    end,
-
-    Pid2 = erlang:spawn(?MODULE, test_linked, [Port]),
-    receive after 200 -> ok end,  % Wait for completion.
-    case is_up(Pid2) of
-        true ->
-            io:format("FAIL.  Linked owner still alive.~n");
-        false ->
-            io:format("PASS.  Linked owner is dead.~n")
-    end,
-
-    Pid3 = erlang:spawn(?MODULE, test_tethered, [Port]),
-    receive after 200 -> ok end,  % Wait for completion.
-    case is_up(Pid3) of
-        true ->
-            io:format("PASS.  Tethered owner still alive.~n");
-        false ->
-            io:format("FAIL.  Tethered owner is dead.~n")
-    end,
-
-    check_extras(3).
-
-is_up(Pid) ->
-    MonitorRef = erlang:monitor(process, Pid),
-    receive
-        {'DOWN', MonitorRef, process, Pid, _Info} ->
-            false
-    after
-        50 ->
-            erlang:demonitor(MonitorRef),
-            true
-    end.
-
-wait_for(Pid) ->
-    MonitorRef = erlang:monitor(process, Pid),
-    receive
-        {'DOWN', MonitorRef, process, Pid, _Info} ->
-            ok
-    end.
-
-check_extras(0) -> ok;
-check_extras(N) ->
-    receive
-        {client, Type, Pid} ->
-            case {Type, is_up(Pid)} of
-                {unlinked, true} ->
-                    io:format("PASS.  Unlinked client still alive.~n");
-                {unlinked, false} ->
-                    io:format("FAIL.  Unlinked client dead.~n");
-                {linked, true} ->
-                    io:format("FAIL.  Linked client still alive.~n");
-                {linked, false} ->
-                    io:format("PASS.  Linked client dead.~n");
-                {tethered, true} ->
-                    io:format("FAIL.  Tethered client still alive.~n");
-                {tethered, false} ->
-                    io:format("PASS.  Tethered client dead.~n")
-            end,
-            check_extras(N-1)
-    after
-        500 ->
-            io:format("FAIL.  Expected ~p more clients.~n", [N])
-    end.
-
-make_thrift_client(Opts) ->
-     thrift_client:start(fun()->ok end, thriftTest_thrift, Opts).
-
-make_protocol_factory(Port) ->
-    {ok, TransportFactory} =
-        thrift_socket_transport:new_transport_factory(
-          "127.0.0.1", Port, []),
-    {ok, ProtocolFactory} =
-        thrift_binary_protocol:new_protocol_factory(
-          TransportFactory, []),
-    ProtocolFactory.
-
-
-test_start(Port) ->
-    {ok, Client1} = make_thrift_client([{connect, false}]),
-    tester ! {client, unlinked, Client1},
-    {ok, Client2} = make_thrift_client([{connect, false}]),
-    io:format("PASS.  Unlinked clients created.~n"),
-    try
-        gen_server:call(Client2, {connect, make_protocol_factory(Port)}),
-        thrift_client:call(Client2, testVoid, []),
-        io:format("FAIL.  Unlinked client connected and called.~n", [])
-    catch
-        Kind:Info ->
-            io:format("PASS.  Caught unlinked error.  ~p:~p~n", [Kind, Info])
-    end,
-    receive after 100 ->
-                    io:format("PASS.  Still alive after unlinked death.~n"),
-                    %% Hang around a little longer so our parent can verify.
-                    receive after 200 -> ok end
-    end,
-    %% Exit abnormally to not kill our unlinked extra client.
-    exit(die).
-
-test_linked(Port) ->
-    {ok, Client1} = make_thrift_client([{connect, false}, {monitor, link}]),
-    tester ! {client, linked, Client1},
-    {ok, Client2} = make_thrift_client([{connect, false}, {monitor, link}]),
-    io:format("PASS.  Linked clients created.~n"),
-    try
-        gen_server:call(Client2, {connect, make_protocol_factory(Port)}),
-        thrift_client:call(Client2, testVoid, []),
-        io:format("FAIL.  Linked client connected and called.~n", [])
-    catch
-        Kind:Info ->
-            io:format("FAIL.  Caught linked error.  ~p:~p~n", [Kind, Info])
-    end,
-    receive after 100 ->
-                    io:format("FAIL.  Still alive after linked death.~n"),
-                    % Hang around a little longer so our parent can verify.
-                    receive after 200 -> ok end
-    end,
-    %% Exit abnormally to kill our linked extra client.
-    %% But we should never get here.
-    exit(die).
-
-test_tethered(Port) ->
-    {ok, Client1} = make_thrift_client([{connect, false}, {monitor, tether}]),
-    tester ! {client, tethered, Client1},
-    {ok, Client2} = make_thrift_client([{connect, false}, {monitor, tether}]),
-    io:format("PASS.  Tethered clients created.~n"),
-    try
-        gen_server:call(Client2, {connect, make_protocol_factory(Port)}),
-        thrift_client:call(Client2, testVoid, []),
-        io:format("FAIL.  Tethered client connected and called.~n", [])
-    catch
-        Kind:Info ->
-            io:format("PASS.  Caught tethered error.  ~p:~p~n", [Kind, Info])
-    end,
-    receive after 100 ->
-                    io:format("PASS.  Still alive after tethered death.~n"),
-                    % Hang around a little longer so our parent can verify.
-                    receive after 200 -> ok end
-    end,
-    %% Exit abnormally to kill our tethered extra client.
-    exit(die).