Merge pull request #3572 from esl/cache_maps

Migrate segmented cache to maps

big_tests/tests/domain_removal_SUITE.erl

@@ -82,7 +82,7 @@ end_per_group(_Groupname, Config) ->
 group_to_modules(auth_removal) ->
     [];
 group_to_modules(cache_removal) ->
-    [{mod_cache_users, []},
+    [{mod_cache_users, config_parser_helper:default_mod_config(mod_cache_users)},
      {mod_mam_meta, mam_helper:config_opts(#{pm => #{}})}];
 group_to_modules(mam_removal) ->
     MucHost = subhost_pattern(muc_light_helper:muc_host_pattern()),

big_tests/tests/muc_light_SUITE.erl

@@ -300,10 +300,10 @@ schema_opts(CaseName) ->
 common_muc_light_opts() ->
     [{host, subhost_pattern(muc_light_helper:muc_host_pattern())},
      {backend, mongoose_helper:mnesia_or_rdbms_backend()},
-     {cache_affs, [{module, internal},
-                   {strategy, fifo},
-                   {time_to_live, 2},
-                   {number_of_segments, 3}]},
+     {cache_affs, #{module => internal,
+                    strategy => fifo,
+                    time_to_live => 2,
+                    number_of_segments => 3}},
      {rooms_in_rosters, true}].
 
 %%--------------------------------------------------------------------

doc/modules/mod_mam.md

@@ -223,7 +223,7 @@ modules.mod_mam_meta.cache.number_of_segments
 * **Default:** `internal`
 * **Example:** `modules.mod_mam_meta.cache.module = "mod_cache_users"`
 
-Configures which cache to use, either start an internal instance, or reuse the cache created by `mod_cache_users`, if such module was enabled. Note that if reuse is desired – that is, `cache.module = "mod_cache_users"`, other cache configuration parameters are not allowed.
+Configures which cache to use, either start an internal instance, or reuse the cache created by `mod_cache_users`, if such module was enabled. Note that if reuse is desired – that is, `cache.module = "mod_cache_users"`, other cache configuration parameters will be ignored.
 
 #### `modules.mod_mam_meta.async_writer.enabled`
 * **Syntax:** boolean

src/mod_cache_users.erl

@@ -33,8 +33,10 @@ stop(HostType) ->
 
 -spec config_spec() -> mongoose_config_spec:config_section().
 config_spec() ->
-    Sec = #section{items = Items} = mongoose_user_cache:config_spec(),
-    Sec#section{items = maps:remove(<<"module">>, Items)}.
+    #section{items = Items,
+             defaults = Defaults} = Sec = mongoose_user_cache:config_spec(),
+    Sec#section{items = maps:remove(<<"module">>, Items),
+                defaults = maps:remove(<<"module">>, Defaults)}.
 
 -spec supported_features() -> [atom()].
 supported_features() ->

src/mongoose_user_cache.erl

@@ -41,16 +41,21 @@ config_spec() ->
                  <<"time_to_live">> => #option{type = int_or_infinity, validate = positive},
                  <<"number_of_segments">> => #option{type = integer, validate = positive}
                 },
-       process = fun ?MODULE:process_cache_config/1
+       process = fun ?MODULE:process_cache_config/1,
+       defaults = #{<<"module">> => internal,
+                    <<"strategy">> => fifo,
+                    <<"time_to_live">> => 480,
+                    <<"number_of_segments">> => 3},
+       format_items = map
       }.
 
 %% If an external module is provided, disallow any other configuration key
-process_cache_config(KVs) ->
-    case lists:keyfind(module, 1, KVs) of
-        {module, Module} when Module =/= internal -> [{module, _}] = KVs;
-        _ -> ok
-    end,
-    KVs.
+process_cache_config(#{module := internal} = Config) ->
+    Config;
+process_cache_config(#{module := Module}) ->
+    #{module => Module};
+process_cache_config(Config) ->
+    Config.
 
 -spec start_new_cache(mongooseim:host_type(), module(), gen_mod:module_opts()) -> any().
 start_new_cache(HostType, Module, Opts) ->
@@ -62,9 +67,9 @@ start_new_cache(HostType, Module, Opts) ->
 do_start_new_cache(HostType, Module, Opts) ->
     CacheName = gen_mod:get_module_proc(HostType, Module),
     CacheOpts = #{merger_fun => fun maps:merge/2,
-                  segment_num => gen_mod:get_opt(number_of_segments, Opts, 3),
-                  strategy => gen_mod:get_opt(strategy, Opts, fifo),
-                  ttl => gen_mod:get_opt(time_to_live, Opts, {hours, 8})},
+                  segment_num => gen_mod:get_opt(number_of_segments, Opts),
+                  strategy => gen_mod:get_opt(strategy, Opts),
+                  ttl => gen_mod:get_opt(time_to_live, Opts)},
     Spec = #{id => CacheName, start => {segmented_cache, start_link, [CacheName, CacheOpts]},
              restart => permanent, shutdown => 5000,
              type => worker, modules => [segmented_cache]},

src/muc_light/mod_muc_light.erl

@@ -218,7 +218,7 @@ config_spec() ->
     #section{
        items = #{<<"backend">> => #option{type = atom,
                                           validate = {module, mod_muc_light_db}},
-                 <<"cache_affs">> => mongoose_user_cache:config_spec(),
+                 <<"cache_affs">> => cache_config_spec(),
                  <<"host">> => #option{type = string,
                                        validate = subdomain_template,
                                        process = fun mongoose_subdomain_utils:make_subdomain_pattern/1},
@@ -239,6 +239,10 @@ config_spec() ->
                 }
       }.
 
+cache_config_spec() ->
+    Sec = mongoose_user_cache:config_spec(),
+    Sec#section{defaults = #{}}.
+
 config_schema_spec() ->
     #section{
        items = #{<<"field">> => #option{type = binary,

src/muc_light/mod_muc_light_cache.erl

@@ -123,7 +123,7 @@ force_clear(HostType) ->
 %%====================================================================
 -spec start_cache(mongooseim:host_type(), gen_mod:module_opts()) -> any().
 start_cache(HostType, Opts) ->
-    FinalOpts = maps:to_list(maps:merge(defaults(), maps:from_list(Opts))),
+    FinalOpts = maps:merge(defaults(), Opts),
     mongoose_user_cache:start_new_cache(HostType, ?MODULE, FinalOpts).
 
 defaults() ->

test/batches_SUITE.erl

@@ -58,23 +58,32 @@ end_per_testcase(_TestCase, _Config) ->
     meck:unload(gen_mod),
     ok.
 
+cache_config() ->
+    config_parser_helper:default_mod_config(mod_cache_users).
+
+cache_config(internal) ->
+    Def = config_parser_helper:default_mod_config(mod_cache_users),
+    Def#{module => internal};
+cache_config(Module) ->
+    #{module => Module}.
+
 %% Tests
 internal_starts_another_cache(_) ->
-    mongoose_user_cache:start_new_cache(host_type(), ?mod(1), []),
-    mongoose_user_cache:start_new_cache(host_type(), ?mod(2), [{module, internal}]),
+    mongoose_user_cache:start_new_cache(host_type(), ?mod(1), cache_config()),
+    mongoose_user_cache:start_new_cache(host_type(), ?mod(2), cache_config(internal)),
     L = [S || S = {_Name, _Pid, worker, [segmented_cache]} <- supervisor:which_children(ejabberd_sup)],
     ?assertEqual(2, length(L)).
 
 external_does_not_start_another_cache(_) ->
-    mongoose_user_cache:start_new_cache(host_type(), ?mod(1), []),
-    mongoose_user_cache:start_new_cache(host_type(), ?mod(2), [{module, ?mod(1)}]),
+    mongoose_user_cache:start_new_cache(host_type(), ?mod(1), cache_config()),
+    mongoose_user_cache:start_new_cache(host_type(), ?mod(2), cache_config(?mod(1))),
     L = [S || S = {_Name, _Pid, worker, [segmented_cache]} <- supervisor:which_children(ejabberd_sup)],
     ?assertEqual(1, length(L)).
 
 internal_stop_does_stop_the_cache(_) ->
     meck:expect(gen_mod, get_module_opt, fun(_, _, module, _) -> internal end),
-    mongoose_user_cache:start_new_cache(host_type(), ?mod(1), []),
-    mongoose_user_cache:start_new_cache(host_type(), ?mod(2), [{module, internal}]),
+    mongoose_user_cache:start_new_cache(host_type(), ?mod(1), cache_config()),
+    mongoose_user_cache:start_new_cache(host_type(), ?mod(2), cache_config(internal)),
     L1 = [S || S = {_Name, _Pid, worker, [segmented_cache]} <- supervisor:which_children(ejabberd_sup)],
     ct:pal("Value ~p~n", [L1]),
     mongoose_user_cache:stop_cache(host_type(), ?mod(2)),
@@ -84,17 +93,17 @@ internal_stop_does_stop_the_cache(_) ->
 
 external_stop_does_nothing(_) ->
     meck:expect(gen_mod, get_module_opt, fun(_, _, module, _) -> ?mod(1) end),
-    mongoose_user_cache:start_new_cache(host_type(), ?mod(1), []),
-    mongoose_user_cache:start_new_cache(host_type(), ?mod(2), [{module, ?mod(1)}]),
+    mongoose_user_cache:start_new_cache(host_type(), ?mod(1), cache_config()),
+    mongoose_user_cache:start_new_cache(host_type(), ?mod(2), cache_config(?mod(1))),
     L1 = [S || S = {_Name, _Pid, worker, [segmented_cache]} <- supervisor:which_children(ejabberd_sup)],
     mongoose_user_cache:stop_cache(host_type(), ?mod(2)),
     L2 = [S || S = {_Name, _Pid, worker, [segmented_cache]} <- supervisor:which_children(ejabberd_sup)],
     ?assertEqual(L1, L2).
 
 shared_cache_inserts_in_shared_table(_) ->
     meck:expect(gen_mod, get_module_opt, fun(_, _, module, _) -> ?mod(1) end),
-    mongoose_user_cache:start_new_cache(host_type(), ?mod(1), []),
-    mongoose_user_cache:start_new_cache(host_type(), ?mod(2), [{module, ?mod(1)}]),
+    mongoose_user_cache:start_new_cache(host_type(), ?mod(1), cache_config()),
+    mongoose_user_cache:start_new_cache(host_type(), ?mod(2), cache_config(?mod(1))),
     mongoose_user_cache:merge_entry(host_type(), ?mod(2), some_jid(), #{}),
     ?assert(mongoose_user_cache:is_member(host_type(), ?mod(1), some_jid())).
 

test/common/config_parser_helper.erl

@@ -399,7 +399,7 @@ all_modules() ->
                        host => {fqdn, <<"muc.example.com">>},
                        no_stanzaid_element => true}),
       mod_caps => [{cache_life_time, 86}, {cache_size, 1000}],
-      mod_mam_cache_user => #{cache => [], muc => true, pm => true},
+      mod_mam_cache_user => #{cache => default_config([modules, mod_mam_meta, cache]), muc => true, pm => true},
       mod_offline =>
           [{access_max_user_messages, max_user_offline_messages},
            {backend, riak},
@@ -828,6 +828,8 @@ default_pool_conn_opts(_Type) ->
 mod_config(Module, ExtraOpts) ->
     maps:merge(default_mod_config(Module), ExtraOpts).
 
+default_mod_config(mod_cache_users) ->
+    #{strategy => fifo, time_to_live => 480, number_of_segments => 3};
 default_mod_config(mod_adhoc) ->
     #{iqdisc => one_queue, report_commands_node => false};
 default_mod_config(mod_auth_token) ->
@@ -875,7 +877,8 @@ default_mod_config(mod_vcard) ->
 default_mod_config(mod_version) ->
     #{iqdisc => no_queue, os_info => false};
 default_mod_config(mod_mam_meta) ->
-    (common_mam_config())#{backend => rdbms, cache_users => true, cache => []};
+    (common_mam_config())#{backend => rdbms, cache_users => true,
+                           cache => default_config([modules, mod_mam_meta, cache])};
 default_mod_config(mod_mam) ->
     maps:merge(common_mam_config(), default_config([modules, mod_mam_meta, pm]));
 default_mod_config(mod_mam_muc) ->
@@ -895,6 +898,9 @@ default_config([modules, mod_mam_meta, pm]) ->
     #{archive_groupchats => false, same_mam_id_for_peers => false};
 default_config([modules, mod_mam_meta, muc]) ->
     #{host => {prefix, <<"conference.">>}};
+default_config([modules, mod_mam_meta, cache]) ->
+    #{module => internal, strategy => fifo,
+      time_to_live => 480, number_of_segments => 3};
 default_config([modules, mod_mam_meta, async_writer]) ->
     #{batch_size => 30, enabled => true, flush_interval => 2000,
       pool_size => 4 * erlang:system_info(schedulers_online)};

test/config_parser_SUITE.erl

@@ -1634,12 +1634,13 @@ mod_caps(_Config) ->
     ?errh(T(<<"cache_life_time">>, <<"infinity">>)).
 
 mod_cache_users(_Config) ->
+    check_module_defaults(mod_cache_users),
+    P = [modules, mod_cache_users],
     T = fun(K, V) -> #{<<"modules">> => #{<<"mod_cache_users">> => #{K => V}}} end,
-    M = fun(K, V) -> modopts(mod_cache_users, [{K, V}]) end,
-    ?cfgh(M(time_to_live, 8600), T(<<"time_to_live">>, 8600)),
-    ?cfgh(M(time_to_live, infinity), T(<<"time_to_live">>, <<"infinity">>)),
-    ?cfgh(M(number_of_segments, 10), T(<<"number_of_segments">>, 10)),
-    ?cfgh(M(strategy, fifo), T(<<"strategy">>, <<"fifo">>)),
+    ?cfgh(P ++ [time_to_live], 8600, T(<<"time_to_live">>, 8600)),
+    ?cfgh(P ++ [time_to_live], infinity, T(<<"time_to_live">>, <<"infinity">>)),
+    ?cfgh(P ++ [number_of_segments], 10, T(<<"number_of_segments">>, 10)),
+    ?cfgh(P ++ [strategy], fifo, T(<<"strategy">>, <<"fifo">>)),
     ?errh(T(<<"time_to_live">>, 0)),
     ?errh(T(<<"strategy">>, <<"lifo">>)),
     ?errh(T(<<"number_of_segments">>, 0)),
@@ -2135,8 +2136,7 @@ mod_mam_meta(_Config) ->
     check_module_defaults(mod_mam_meta),
     T = fun(Opts) -> #{<<"modules">> => #{<<"mod_mam_meta">> => Opts}} end,
     P = [modules, mod_mam_meta],
-    test_segmented_cache_config(<<"cache">>, cache, T,
-                                fun([{cache, Opts}]) -> [{P ++ [cache], Opts}] end),
+    test_cache_config(T, P),
     test_mod_mam_meta(T, P).
 
 mod_mam_meta_riak(_Config) ->
@@ -2221,26 +2221,33 @@ test_mod_mam_meta(T, P) ->
     ?errh(T(#{<<"extra_fin_element">> => <<"bad_module">>})),
     ?errh(T(#{<<"extra_lookup_params">> => <<"bad_module">>})).
 
-test_cache_config(T, M) ->
-    ?cfgh(M([{cache_users, false}]),
-          T(#{<<"cache_users">> => false})),
-    ?errh(T(#{<<"cache_users">> => []})),
-    test_segmented_cache_config(<<"cache">>, cache, T, M).
+test_cache_config(ParentT, ParentP) ->
+    P = ParentP ++ [cache],
+    T = fun(Opts) -> ParentT(#{<<"cache">> => Opts}) end,
+    ?cfgh(P ++ [module], internal, T(#{<<"module">> => <<"internal">>})),
+    ?cfgh(P ++ [time_to_live], 8600, T(#{<<"time_to_live">> => 8600})),
+    ?cfgh(P ++ [time_to_live], infinity, T(#{<<"time_to_live">> => <<"infinity">>})),
+    ?cfgh(P ++ [number_of_segments], 10, T(#{<<"number_of_segments">> => 10})),
+    ?cfgh(P ++ [strategy], fifo, T(#{<<"strategy">> => <<"fifo">>})),
+    ?errh(T(#{<<"module">> => <<"mod_wrong_cache">>})),
+    ?errh(T(#{<<"time_to_live">> => 0})),
+    ?errh(T(#{<<"strategy">> => <<"lifo">>})),
+    ?errh(T(#{<<"number_of_segments">> => 0})),
+    ?errh(T(#{<<"number_of_segments">> => <<"infinity">>})),
+    ?errh(T(#{<<"cache">> => []})).
 
 test_segmented_cache_config(NameK, NameV, T, M) ->
-    ?cfgh(M([{NameV, [{module, internal}]}]),
+    ?cfgh(M([{NameV, #{module => internal}}]),
           T(#{NameK => #{<<"module">> => <<"internal">>}})),
-    ?cfgh(M([{NameV, [{time_to_live, 8600}]}]),
+    ?cfgh(M([{NameV, #{time_to_live => 8600}}]),
           T(#{NameK => #{<<"time_to_live">> => 8600}})),
-    ?cfgh(M([{NameV, [{time_to_live, infinity}]}]),
+    ?cfgh(M([{NameV, #{time_to_live => infinity}}]),
           T(#{NameK => #{<<"time_to_live">> => <<"infinity">>}})),
-    ?cfgh(M([{NameV, [{number_of_segments, 10}]}]),
+    ?cfgh(M([{NameV, #{number_of_segments => 10}}]),
           T(#{NameK => #{<<"number_of_segments">> => 10}})),
-    ?cfgh(M([{NameV, [{strategy, fifo}]}]),
+    ?cfgh(M([{NameV, #{strategy => fifo}}]),
           T(#{NameK => #{<<"strategy">> => <<"fifo">>}})),
     ?errh(T(#{NameK => #{<<"module">> => <<"mod_wrong_cache">>}})),
-    ?errh(T(#{NameK => #{<<"module">> => <<"mod_cache_users">>,
-                               <<"time_to_live">> => 8600}})),
     ?errh(T(#{NameK => #{<<"time_to_live">> => 0}})),
     ?errh(T(#{NameK => #{<<"strategy">> => <<"lifo">>}})),
     ?errh(T(#{NameK => #{<<"number_of_segments">> => 0}})),

test/mod_mam_meta_SUITE.erl

@@ -68,14 +68,15 @@ produces_valid_configurations(_Config) ->
     MUC = config([modules, mod_mam_meta, muc],
                  maps:merge(MUCCoreOpts, MUCArchOpts#{user_prefs_store => mnesia})),
     Deps = deps(#{pm => PM, muc => MUC}),
+    Cache = default_config([modules, mod_mam_meta, cache]),
 
     check_equal_opts(mod_mam, mod_config(mod_mam, PMCoreOpts), Deps),
     check_equal_opts(mod_mam_muc, mod_config(mod_mam_muc, MUCCoreOpts), Deps),
     check_equal_opts(mod_mam_rdbms_arch, default_mod_config(mod_mam_rdbms_arch), Deps),
     check_equal_opts(mod_mam_muc_rdbms_arch, mod_config(mod_mam_muc_rdbms_arch,
                                                         MUCArchOpts#{no_writer => true}), Deps),
     check_equal_opts(mod_mam_rdbms_user, #{pm => true, muc => true}, Deps),
-    check_equal_opts(mod_mam_cache_user, #{pm => true, muc => true, cache => []}, Deps),
+    check_equal_opts(mod_mam_cache_user, #{pm => true, muc => true, cache => Cache}, Deps),
     check_equal_opts(mod_mam_mnesia_prefs, #{muc => true}, Deps),
     check_equal_opts(mod_mam_rdbms_prefs, #{pm => true}, Deps),
     check_equal_opts(mod_mam_muc_rdbms_arch_async, AsyncOpts, Deps).
@@ -111,10 +112,11 @@ example_muc_only_no_pref_good_performance(_Config) ->
     MUC = config([modules, mod_mam_meta, muc], MUCOpts),
     Deps = deps(#{muc => MUC}),
     AsyncOpts = default_config([modules, mod_mam_meta, async_writer]),
+    Cache = default_config([modules, mod_mam_meta, cache]),
 
     check_equal_deps(
       [{mod_mam_rdbms_user, #{muc => true, pm => true}},
-       {mod_mam_cache_user, #{muc => true, cache => []}},
+       {mod_mam_cache_user, #{muc => true, cache => Cache}},
        {mod_mam_muc_rdbms_arch, mod_config(mod_mam_muc_rdbms_arch, #{no_writer => true})},
        {mod_mam_muc_rdbms_arch_async, AsyncOpts},
        {mod_mam_muc, mod_config(mod_mam_muc, mod_config(mod_mam_muc, MUCOpts))}
@@ -124,10 +126,11 @@ example_pm_only_good_performance(_Config) ->
     PM = default_config([modules, mod_mam_meta, pm]),
     Deps = deps(#{pm => PM, user_prefs_store => mnesia}),
     AsyncOpts = default_config([modules, mod_mam_meta, async_writer]),
+    Cache = default_config([modules, mod_mam_meta, cache]),
 
     check_equal_deps(
       [{mod_mam_rdbms_user, #{pm => true}},
-       {mod_mam_cache_user, #{pm => true, cache => []}},
+       {mod_mam_cache_user, #{pm => true, cache => Cache}},
        {mod_mam_mnesia_prefs, #{pm => true}},
        {mod_mam_rdbms_arch, mod_config(mod_mam_rdbms_arch, #{no_writer => true})},
        {mod_mam_rdbms_arch_async, AsyncOpts},