[snmp/agent] Fixed request id generation

When testing the agent, it is started and then used for a series
(v1, v2, v3, ...) of tests. But the individual test cases are
use one instance of the test manager. Because of this, the requerst
id cannot be a counter starting from 1 or any counter for that
matter. Instead every request id was generated using random.
But this is obiously not a guarantee fo uniqueness, it merely
makes it unlikely that the request ids will be reused.
To get around this problem (without having to rewrite the
entire agent test suite) a simple (global) counter server is
introduced. It is started at the start of the agent test suite
(init_per_suite) and stopped at the end (end_per_suite).

4 files changed, 169 insertions, 2 deletions

diff --git a/lib/snmp/test/modules.mk b/lib/snmp/test/modules.mk
index 3d658bf8e8..fd8315ec4d 100644
--- a/lib/snmp/test/modules.mk
+++ b/lib/snmp/test/modules.mk
@@ -2,7 +2,7 @@
 
 # %CopyrightBegin%
 #
-# Copyright Ericsson AB 2004-2012. All Rights Reserved.
+# Copyright Ericsson AB 2004-2014. All Rights Reserved.
 #
 # The contents of this file are subject to the Erlang Public License,
 # Version 1.1, (the "License"); you may not use this file except in
@@ -42,6 +42,7 @@ TEST_UTIL_MODULES = \
 	snmp_test_manager \
 	snmp_test_mgr \
 	snmp_test_mgr_misc \
+	snmp_test_mgr_counter_server \
 	sa \
 	klas3 \
 	test1 \
diff --git a/lib/snmp/test/snmp_agent_test.erl b/lib/snmp/test/snmp_agent_test.erl
index 84fa1af71d..a6571548d6 100644
--- a/lib/snmp/test/snmp_agent_test.erl
+++ b/lib/snmp/test/snmp_agent_test.erl
@@ -555,6 +555,8 @@ init_per_suite(Config0) when is_list(Config0) ->
 
     Config3 = [{mib_dir, MibDir}, {std_mib_dir, StdMibDir} | Config2],
 
+    snmp_test_mgr_counter_server:start(), 
+
     ?DBG("init_per_suite -> end with"
 	 "~n   Config3: ~p", [Config3]),
 
@@ -565,6 +567,17 @@ end_per_suite(Config) when is_list(Config) ->
     ?DBG("end_per_suite -> entry with"
 	 "~n   Config: ~p", [Config]),
 
+    case snmp_test_mgr_counter_server:stop() of
+    	{ok, Counters} ->
+    	    ?DBG("end_per_suite -> sucessfully stopped counter server"
+    		 "~n   Counters: ~p", [Counters]);
+    
+    	{error, Reason} ->
+    	    ?DBG("end_per_suite -> failed stopping counter server"
+    		 "~n   Reason: ~p", [Reason])
+    end,
+
+    ?DBG("end_per_suite -> end", []),
     Config.
 
 
diff --git a/lib/snmp/test/snmp_test_mgr.erl b/lib/snmp/test/snmp_test_mgr.erl
index 40fcbce8f1..d4eb00ff91 100644
--- a/lib/snmp/test/snmp_test_mgr.erl
+++ b/lib/snmp/test/snmp_test_mgr.erl
@@ -657,7 +657,8 @@ make_vb(Oid) ->
     #varbind{oid = Oid, variabletype = 'NULL', value = 'NULL'}.
 
 make_request_id() ->
-    random:uniform(16#FFFFFFF-1).
+    %% random:uniform(16#FFFFFFF-1).
+    snmp_test_mgr_counter_server:increment(mgr_request_id, 1, 1, 2147483647).
 
 echo_pdu(PDU, MiniMIB) ->
     io:format("~s", [snmp_misc:format_pdu(PDU, MiniMIB)]).
diff --git a/lib/snmp/test/snmp_test_mgr_counter_server.erl b/lib/snmp/test/snmp_test_mgr_counter_server.erl
new file mode 100644
index 0000000000..db31e0380b
--- /dev/null
+++ b/lib/snmp/test/snmp_test_mgr_counter_server.erl
@@ -0,0 +1,152 @@
+%% 
+%% %CopyrightBegin%
+%%
+%% Copyright Ericsson AB 2014-2014. All Rights Reserved.
+%%
+%% The contents of this file are subject to the Erlang Public License,
+%% Version 1.1, (the "License"); you may not use this file except in
+%% compliance with the License. You should have received a copy of the
+%% Erlang Public License along with this software. If not, it can be
+%% retrieved online at http://www.erlang.org/.
+%%
+%% Software distributed under the License is distributed on an "AS IS"
+%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See
+%% the License for the specific language governing rights and limitations
+%% under the License.
+%%
+%% %CopyrightEnd%
+%% 
+
+%% 
+%% The reason for this (test) counter server is that the 
+%% agent test suite is implemented in such a way that the 
+%% agent is started once and then used for several test cases.
+%% Each request is given a request id which *was* generated using 
+%% random! It is therefor possible, although unlikely, that a 
+%% request may get a request id that has recently been used, 
+%% which will cause the agent to silently reject the request.
+%% For this reason, we start this server at the start of the
+%% agent suite and stop it at the end and all request ids are 
+%% generated by this server.
+%% 
+
+-module(snmp_test_mgr_counter_server).
+
+-export([start/0, stop/0, increment/4]).
+
+-define(SERVER, ?MODULE).
+-define(TAB,    snmp_test_mgr_counter_tab).
+
+
+%%%-------------------------------------------------------------------
+%%% API
+%%%-------------------------------------------------------------------
+
+-spec start() -> ok.
+    
+start() ->
+    Parent      = self(),
+    ReqIdServer = spawn(fun() -> init(Parent) end),
+    receive
+	{ReqIdServer, ok} ->
+	    ok;
+	{ReqIdServer, {error, Reason}} ->
+	    exit({failed_starting_counter_server, Reason})
+    after 5000 ->
+	    exit(ReqIdServer, kill), % Cleanup, just in case
+	    exit({failed_starting_counter_server, timeout})
+    end.
+
+-spec stop() -> {ok, Counters :: list()} | {error, Reason :: term()}.
+    
+stop() ->
+    request(stop).
+
+
+-spec increment(Counter  :: atom(), 
+	       Initial   :: non_neg_integer(), 
+	       Increment :: pos_integer(), 
+	       Max       :: pos_integer()) -> 
+    Next :: pos_integer().
+
+increment(Counter, Initial, Increment, Max) ->
+    Request = {increment, Counter, Initial, Increment, Max}, 
+    case request(Request) of
+	{ok, ReqId} ->
+	    ReqId;
+	{error, Reason} ->
+	    exit(Reason)
+    end.
+
+
+request(Request) ->
+    Id  = make_ref(),
+    Msg = {self(), Id, Request}, 
+    try
+	begin
+	    global:send(?SERVER, Msg),
+	    receive
+		{reply, Id, Reply} ->
+		    {ok, Reply}
+	    end
+	end
+    catch
+	T:E ->
+	    {error, {T, E}}
+    end.
+		
+    
+%%%-------------------------------------------------------------------
+%%% Internal functions
+%%%-------------------------------------------------------------------
+
+init(Parent) ->
+    p("starting"),
+    case global:register_name(?SERVER, self()) of
+	yes ->
+	    p("name registration ok"),
+	    Parent ! {self(), ok};
+	no ->
+	    p("name registration failed"),
+	    Parent ! {self(), registration_failed},
+	    exit(registration_failed)
+    end,
+    ets:new(?TAB, [set, named_table, {keypos, 1}]),
+    loop().
+
+loop() ->
+    receive
+	{From, Id, {increment, Counter, Initial, Increment, Max}} ->
+	    Position  = 2,
+	    Threshold = Max,
+	    SetValue  = Initial,
+	    UpdateOp  = {Position, Increment, Threshold, SetValue},
+	    NextVal = 
+		try ets:update_counter(?TAB, Counter, UpdateOp) of
+		    Next when is_integer(Next) ->
+			p("increment ~w: (next) ~w", [Counter, Next]),
+			Next
+		catch
+		    error:badarg ->
+			%% Oups, first time
+			p("increment ~w: (initial) ~w", [Counter, Initial]),
+			ets:insert(?TAB, {Counter, Initial}),
+			Initial
+		end,
+	    From ! {reply, Id, NextVal},
+	    loop();
+
+	{From, Id, stop} ->
+	    p("stop"),
+	    Counters = ets:tab2list(?TAB), 
+	    From ! {reply, Id, Counters}, 
+	    exit(normal)
+    end.
+
+
+p(F) ->
+    p(F, []).
+
+p(F, A) ->
+    io:format("*** [~s] COUNTER-SERVER [~w] " ++ F ++ "~n", 
+	      [snmp_test_lib:formated_timestamp(), self() | A]).