Limit the access to the `#pool` record

this is to make it safer to upgrade the record

src/pooler.erl

@@ -65,7 +65,7 @@
 -export_type([pool_config/0, pool_name/0, group_name/0, member_info/0, time_unit/0, time_spec/0]).
 
 % Internal
--export_type([members_map/0, consumers_map/0, requestor_queue/0]).
+-export_type([members_map/0, consumers_map/0, requestor_queue/0, pool_state/0]).
 
 -type pool_name() :: atom().
 %% The name of the pool
@@ -99,6 +99,8 @@
 %% Internal
 -type consumers_map() :: #{pid() => {reference(), [pid()]}}.
 %% Internal
+-type pool_state() :: #pool{}.
+%% Internal
 
 -if(?OTP_RELEASE >= 25).
 -type gen_server_from() :: gen_server:from().
@@ -219,8 +221,7 @@ rm_group(GroupName) ->
 rm_group_members(MemberPids) ->
     lists:foldl(
         fun(MemberPid, Acc) ->
-            Pool = gen_server:call(MemberPid, dump_pool),
-            PoolName = Pool#pool.name,
+            #{name := PoolName} = gen_server:call(MemberPid, dump_pool),
             case pooler_sup:rm_pool(PoolName) of
                 ok -> Acc;
                 _ -> [PoolName | Acc]
@@ -439,7 +440,7 @@ handle_call(pool_stats, _From, Pool) ->
 handle_call(pool_utilization, _From, Pool) ->
     {reply, compute_utilization(Pool), Pool};
 handle_call(dump_pool, _From, Pool) ->
-    {reply, Pool, Pool};
+    {reply, to_map(Pool), Pool};
 handle_call({call_free_members, Fun}, _From, #pool{free_pids = Pids} = Pool) ->
     {reply, do_call_free_members(Fun, Pids), Pool};
 handle_call(_Request, _From, Pool) ->
@@ -1200,6 +1201,12 @@ do_call_free_member(Fun, Pid) ->
             {error, Reason}
     end.
 
+to_map(#pool{} = Pool) ->
+    [Name | Values] = tuple_to_list(Pool),
+    maps:from_list(
+        [{'$record_name', Name} | lists:zip(record_info(fields, pool), Values)]
+    ).
+
 % >= OTP-21
 -ifdef(OTP_RELEASE).
 -if(?OTP_RELEASE >= 23).

src/pooler_config.erl

@@ -4,7 +4,7 @@
 
 -module(pooler_config).
 
--export([list_to_pool/1]).
+-export([list_to_pool/1, get_name/1, get_start_mfa/1, group_table/0]).
 
 -include("pooler.hrl").
 
@@ -27,6 +27,18 @@ list_to_pool(P) ->
         queue_max = ?gv(queue_max, P, ?DEFAULT_POOLER_QUEUE_MAX)
     }.
 
+-spec get_name(#pool{}) -> pooler:pool_name().
+get_name(#pool{name = Name}) ->
+    Name.
+
+-spec get_start_mfa(pooler:pool_state()) -> {module(), atom(), [any()]}.
+get_start_mfa(#pool{start_mfa = MFA}) ->
+    MFA.
+
+-spec group_table() -> atom().
+group_table() ->
+    ?POOLER_GROUP_TABLE.
+
 %% Return `Value' for `Key' in proplist `P' or crashes with an
 %% informative message if no value is found.
 req(Key, P) ->

src/pooler_pool_sup.erl

@@ -10,13 +10,12 @@
     build_member_sup_name/1
 ]).
 
--include("pooler.hrl").
-
-start_link(#pool{} = Pool) ->
+-spec start_link(pooler:pool_state()) -> {ok, pid()}.
+start_link(Pool) ->
     SupName = pool_sup_name(Pool),
     supervisor:start_link({local, SupName}, ?MODULE, Pool).
 
-init(#pool{} = Pool) ->
+init(Pool) ->
     PoolerSpec = {pooler, {pooler, start_link, [Pool]}, transient, 5000, worker, [pooler]},
     MemberSupName = member_sup_name(Pool),
     MemberSupSpec =
@@ -28,11 +27,11 @@ init(#pool{} = Pool) ->
     Restart = {one_for_all, 5, 60},
     {ok, {Restart, [MemberSupSpec, PoolerSpec]}}.
 
-member_sup_name(#pool{name = PoolName}) ->
-    build_member_sup_name(PoolName).
+member_sup_name(Pool) ->
+    build_member_sup_name(pooler_config:get_name(Pool)).
 
 build_member_sup_name(PoolName) ->
     list_to_atom("pooler_" ++ atom_to_list(PoolName) ++ "_member_sup").
 
-pool_sup_name(#pool{name = PoolName}) ->
-    list_to_atom("pooler_" ++ atom_to_list(PoolName) ++ "_pool_sup").
+pool_sup_name(Pool) ->
+    list_to_atom("pooler_" ++ atom_to_list(pooler_config:get_name(Pool)) ++ "_pool_sup").

src/pooler_pooled_worker_sup.erl

@@ -4,9 +4,8 @@
 
 -export([start_link/1, init/1]).
 
--include("pooler.hrl").
-
-start_link(#pool{start_mfa = {_, _, _} = MFA} = Pool) ->
+start_link(Pool) ->
+    MFA = pooler_config:get_start_mfa(Pool),
     SupName = pooler_pool_sup:member_sup_name(Pool),
     supervisor:start_link({local, SupName}, ?MODULE, MFA).
 

src/pooler_starter.erl

@@ -5,7 +5,6 @@
 -module(pooler_starter).
 -behaviour(gen_server).
 
--include("pooler.hrl").
 -include_lib("kernel/include/logger.hrl").
 
 %% ------------------------------------------------------------------

src/pooler_sup.erl

@@ -10,8 +10,6 @@
     start_link/0
 ]).
 
--include("pooler.hrl").
-
 start_link() ->
     supervisor:start_link({local, ?MODULE}, ?MODULE, []).
 
@@ -31,7 +29,7 @@ init([]) ->
     ],
     Pools = [pooler_config:list_to_pool(MetricsConfig ++ L) || L <- Config],
     PoolSupSpecs = [pool_sup_spec(Pool) || Pool <- Pools],
-    ets:new(?POOLER_GROUP_TABLE, [set, public, named_table, {write_concurrency, true}]),
+    ets:new(pooler_config:group_table(), [set, public, named_table, {write_concurrency, true}]),
     {ok, {{one_for_one, 5, 60}, [starter_sup_spec() | PoolSupSpecs]}}.
 
 %% @doc Create a new pool from proplist pool config `PoolConfig'. The
@@ -67,8 +65,8 @@ rm_pool(Name) ->
 starter_sup_spec() ->
     {pooler_starter_sup, {pooler_starter_sup, start_link, []}, transient, 5000, supervisor, [pooler_starter_sup]}.
 
-pool_sup_spec(#pool{name = Name} = Pool) ->
-    SupName = pool_sup_name(Name),
+pool_sup_spec(Pool) ->
+    SupName = pool_sup_name(pooler_config:get_name(Pool)),
     {SupName, {pooler_pool_sup, start_link, [Pool]}, transient, 5000, supervisor, [pooler_pool_sup]}.
 
 pool_sup_name(Name) ->

test/pooler_tests.erl

@@ -1,7 +1,6 @@
 -module(pooler_tests).
 
 -include_lib("eunit/include/eunit.hrl").
--include("../src/pooler.hrl").
 
 -export([sleep_for_configured_timeout/0]).
 
@@ -329,13 +328,13 @@ basic_tests() ->
             M2 = pooler:take_member(test_pool_1),
             ?assert(M1 =/= M2),
             Pool1 = gen_server:call(test_pool_1, dump_pool),
-            ?assertEqual(2, Pool1#pool.in_use_count),
-            ?assertEqual(0, Pool1#pool.free_count),
+            ?assertEqual(2, maps:get(in_use_count, Pool1)),
+            ?assertEqual(0, maps:get(free_count, Pool1)),
             pooler:return_member(test_pool_1, M1),
             pooler:return_member(test_pool_1, M2),
             Pool2 = gen_server:call(test_pool_1, dump_pool),
-            ?assertEqual(0, Pool2#pool.in_use_count),
-            ?assertEqual(2, Pool2#pool.free_count),
+            ?assertEqual(0, maps:get(in_use_count, Pool2)),
+            ?assertEqual(2, maps:get(free_count, Pool2)),
             ok
         end},
 
@@ -435,13 +434,13 @@ basic_tests() ->
         end},
 
         {"utilization returns sane results", fun() ->
-            #pool{max_count = MaxCount, queue_max = QueueMax} = sys:get_state(test_pool_1),
+            #{max_count := MaxCount, queue_max := QueueMax} = dump_pool(test_pool_1),
 
-            ?assertEqual(MaxCount, ?gv(max_count, pooler:pool_utilization(test_pool_1))),
-            ?assertEqual(0, ?gv(in_use_count, pooler:pool_utilization(test_pool_1))),
-            ?assertEqual(2, ?gv(free_count, pooler:pool_utilization(test_pool_1))),
-            ?assertEqual(0, ?gv(queued_count, pooler:pool_utilization(test_pool_1))),
-            ?assertEqual(QueueMax, ?gv(queue_max, pooler:pool_utilization(test_pool_1)))
+            ?assertEqual(MaxCount, proplists:get_value(max_count, pooler:pool_utilization(test_pool_1))),
+            ?assertEqual(0, proplists:get_value(in_use_count, pooler:pool_utilization(test_pool_1))),
+            ?assertEqual(2, proplists:get_value(free_count, pooler:pool_utilization(test_pool_1))),
+            ?assertEqual(0, proplists:get_value(queued_count, pooler:pool_utilization(test_pool_1))),
+            ?assertEqual(QueueMax, proplists:get_value(queue_max, pooler:pool_utilization(test_pool_1)))
         end}
     ].
 
@@ -680,7 +679,7 @@ pooler_scheduled_cull_test_() ->
                             %% wait for longer than cull delay
                             timer:sleep(250),
                             ?assertEqual(2, length(pooler:pool_stats(test_pool_1))),
-                            ?assertEqual(2, ?gv(free_count, pooler:pool_utilization(test_pool_1)))
+                            ?assertEqual(2, proplists:get_value(free_count, pooler:pool_utilization(test_pool_1)))
                         end}
                     end,
 
@@ -690,7 +689,7 @@ pooler_scheduled_cull_test_() ->
                             %% wait for longer than cull delay
                             timer:sleep(250),
                             ?assertEqual(2, length(pooler:pool_stats(test_pool_1))),
-                            ?assertEqual(2, ?gv(free_count, pooler:pool_utilization(test_pool_1)))
+                            ?assertEqual(2, proplists:get_value(free_count, pooler:pool_utilization(test_pool_1)))
                         end}
                     end,
 
@@ -731,8 +730,8 @@ in_use_members_not_culled(Pids, N) ->
             PidCount,
             length(pooler:pool_stats(test_pool_1))
         ),
-        ?assertEqual(0, ?gv(free_count, pooler:pool_utilization(test_pool_1))),
-        ?assertEqual(PidCount, ?gv(in_use_count, pooler:pool_utilization(test_pool_1))),
+        ?assertEqual(0, proplists:get_value(free_count, pooler:pool_utilization(test_pool_1))),
+        ?assertEqual(PidCount, proplists:get_value(in_use_count, pooler:pool_utilization(test_pool_1))),
         Returns = lists:sublist(Pids, N),
         [
             pooler:return_member(test_pool_1, P)
@@ -785,7 +784,7 @@ random_message_test_() ->
 
             fun() ->
                 RawPool = gen_server:call(test_pool_1, dump_pool),
-                ?assertEqual(pool, element(1, RawPool))
+                ?assertEqual(pool, maps:get('$record_name', RawPool))
             end
         ]}.
 
@@ -901,7 +900,7 @@ pooler_integration_queueing_test_() ->
         [
             fun(_) ->
                 fun() ->
-                    ?assertEqual(0, (dump_pool(test_pool_1))#pool.free_count),
+                    ?assertEqual(0, maps:get(free_count, dump_pool(test_pool_1))),
                     Val = pooler:take_member(test_pool_1, 10),
                     ?assert(is_pid(Val)),
                     pooler:return_member(test_pool_1, Val)
@@ -910,10 +909,10 @@ pooler_integration_queueing_test_() ->
             fun(_) ->
                 fun() ->
                     application:set_env(pooler, sleep_time, 1),
-                    ?assertEqual(0, (dump_pool(test_pool_1))#pool.free_count),
+                    ?assertEqual(0, maps:get(free_count, dump_pool(test_pool_1))),
                     Val = pooler:take_member(test_pool_1, 0),
                     ?assertEqual(error_no_members, Val),
-                    ?assertEqual(0, ?gv(free_count, pooler:pool_utilization(test_pool_1))),
+                    ?assertEqual(0, proplists:get_value(free_count, pooler:pool_utilization(test_pool_1))),
                     timer:sleep(50),
                     %Next request should be available
                     Pid = pooler:take_member(test_pool_1, 0),
@@ -924,10 +923,10 @@ pooler_integration_queueing_test_() ->
             fun(_) ->
                 fun() ->
                     application:set_env(pooler, sleep_time, 10),
-                    ?assertEqual(0, (dump_pool(test_pool_1))#pool.free_count),
+                    ?assertEqual(0, maps:get(free_count, dump_pool(test_pool_1))),
                     [
                         ?assertEqual(pooler:take_member(test_pool_1, 0), error_no_members)
-                     || _ <- lists:seq(1, (dump_pool(test_pool_1))#pool.max_count)
+                     || _ <- lists:seq(1, maps:get(max_count, dump_pool(test_pool_1)))
                     ],
                     timer:sleep(50),
                     %Next request should be available
@@ -947,15 +946,15 @@ pooler_integration_queueing_test_() ->
                             ?assert(is_pid(Val)),
                             pooler:return_member(Val)
                         end)
-                     || _ <- lists:seq(1, (dump_pool(test_pool_1))#pool.max_count)
+                     || _ <- lists:seq(1, maps:get(max_count, dump_pool(test_pool_1)))
                     ],
-                    ?assertEqual(0, ?gv(free_count, pooler:pool_utilization(test_pool_1))),
-                    ?assert(?gv(queued_count, pooler:pool_utilization(test_pool_1)) >= 1),
-                    ?assertEqual(10, ?gv(queue_max, pooler:pool_utilization(test_pool_1))),
+                    ?assertEqual(0, proplists:get_value(free_count, pooler:pool_utilization(test_pool_1))),
+                    ?assert(proplists:get_value(queued_count, pooler:pool_utilization(test_pool_1)) >= 1),
+                    ?assertEqual(10, proplists:get_value(queue_max, pooler:pool_utilization(test_pool_1))),
 
                     timer:sleep(50),
-                    ?assertEqual(10, queue:len((dump_pool(test_pool_1))#pool.queued_requestors)),
-                    ?assertEqual(10, ?gv(queue_max, pooler:pool_utilization(test_pool_1))),
+                    ?assertEqual(10, queue:len(maps:get(queued_requestors, dump_pool(test_pool_1)))),
+                    ?assertEqual(10, proplists:get_value(queue_max, pooler:pool_utilization(test_pool_1))),
 
                     ?assertEqual(pooler:take_member(test_pool_1, 500), error_no_members),
                     ExpectKeys = lists:sort([
@@ -1021,7 +1020,7 @@ pooler_integration_queueing_return_member_test_() ->
                             end,
                             Parent ! {returned, self()}
                         end)
-                     || _ <- lists:seq(1, (dump_pool(test_pool_1))#pool.max_count)
+                     || _ <- lists:seq(1, maps:get(max_count, dump_pool(test_pool_1)))
                     ],
                     [
                         receive
@@ -1047,9 +1046,13 @@ pooler_integration_queueing_return_member_test_() ->
                             pooler:return_member(test_pool_1, Result)
                     end,
                     ?assertEqual(
-                        (dump_pool(test_pool_1))#pool.max_count, length((dump_pool(test_pool_1))#pool.free_pids)
+                        maps:get(max_count, dump_pool(test_pool_1)),
+                        length(maps:get(free_pids, dump_pool(test_pool_1)))
                     ),
-                    ?assertEqual((dump_pool(test_pool_1))#pool.max_count, (dump_pool(test_pool_1))#pool.free_count)
+                    ?assertEqual(
+                        maps:get(max_count, dump_pool(test_pool_1)),
+                        maps:get(free_count, dump_pool(test_pool_1))
+                    )
                 end
             end
         ]}.
@@ -1144,11 +1147,11 @@ pooler_auto_grow_disabled_by_default_test_() ->
             end,
             [
                 {"take one, and it should not auto-grow", fun() ->
-                    ?assertEqual(2, (dump_pool(test_pool_1))#pool.free_count),
+                    ?assertEqual(2, maps:get(free_count, dump_pool(test_pool_1))),
                     P = pooler:take_member(test_pool_1),
                     ?assertMatch({"type-0", _Id}, pooled_gs:get_id(P)),
                     timer:sleep(100),
-                    ?assertEqual(1, (dump_pool(test_pool_1))#pool.free_count),
+                    ?assertEqual(1, maps:get(free_count, dump_pool(test_pool_1))),
                     ok,
                     pooler:return_member(test_pool_1, P)
                 end}
@@ -1183,11 +1186,11 @@ pooler_auto_grow_enabled_test_() ->
             end,
             [
                 {"take one, and it should grow by 2", fun() ->
-                    ?assertEqual(2, (dump_pool(test_pool_1))#pool.free_count),
+                    ?assertEqual(2, maps:get(free_count, dump_pool(test_pool_1))),
                     P = pooler:take_member(test_pool_1),
                     ?assertMatch({"type-0", _Id}, pooled_gs:get_id(P)),
                     timer:sleep(100),
-                    ?assertEqual(3, (dump_pool(test_pool_1))#pool.free_count),
+                    ?assertEqual(3, maps:get(free_count, dump_pool(test_pool_1))),
                     ok,
                     pooler:return_member(test_pool_1, P)
                 end}
@@ -1346,13 +1349,13 @@ call_free_members_test_() ->
             end,
             [
                 {"perform a ping across the pool when all workers are free", fun() ->
-                    ?assertEqual(NumWorkers, (dump_pool(PoolName))#pool.free_count),
+                    ?assertEqual(NumWorkers, maps:get(free_count, dump_pool(PoolName))),
                     Res = pooler:call_free_members(PoolName, fun pooled_gs:ping/1),
 
                     ?assertEqual(NumWorkers, count_pongs(Res))
                 end},
                 {"perform a ping across the pool when two workers are taken", fun() ->
-                    ?assertEqual(NumWorkers, (dump_pool(PoolName))#pool.free_count),
+                    ?assertEqual(NumWorkers, maps:get(free_count, dump_pool(PoolName))),
                     Pids = [pooler:take_member(PoolName) || _X <- lists:seq(0, 1)],
                     Res = pooler:call_free_members(PoolName, fun pooled_gs:ping/1),
 
@@ -1361,7 +1364,7 @@ call_free_members_test_() ->
                     [pooler:return_member(PoolName, P) || P <- Pids]
                 end},
                 {"perform an op where the op crashes all free members", fun() ->
-                    ?assertEqual(NumWorkers, (dump_pool(PoolName))#pool.free_count),
+                    ?assertEqual(NumWorkers, maps:get(free_count, dump_pool(PoolName))),
                     Res = pooler:call_free_members(
                         PoolName,
                         fun pooled_gs:error_on_call/1
@@ -1421,7 +1424,7 @@ children_count(SupId) ->
     length(supervisor:which_children(SupId)).
 
 starting_members(PoolName) ->
-    length((dump_pool(PoolName))#pool.starting_members).
+    length(maps:get(starting_members, dump_pool(PoolName))).
 
 dump_pool(PoolName) ->
     gen_server:call(PoolName, dump_pool).

test/prop_pooler.erl

@@ -12,7 +12,6 @@
 
 -include_lib("proper/include/proper.hrl").
 -include_lib("stdlib/include/assert.hrl").
--include("pooler.hrl").
 
 prop_fixed_start(doc) ->
     "Check that the pool of any fixed size can be started, internal statistics is correct".