fix(gateway): always clear expired custom statuses (#2807)

fluxer_gateway/src/presence/presence_broadcast.erl

@@ -269,60 +269,45 @@ dispatch_to_sessions(Payload, State) ->
 -ifdef(TEST).
 -include_lib("eunit/include/eunit.hrl").
 
-expiry_timer_not_armed_while_disabled_test() ->
-    application:unset_env(fluxer_gateway, custom_status_expiry_enabled),
-    State = refresh_custom_status_expiry_timer(#{custom_status => future_custom_status()}),
-    ?assertNot(maps:is_key(?CUSTOM_STATUS_EXPIRY_TIMER, State)).
-
 expiry_timer_not_armed_without_expires_at_test() ->
-    with_expiry_enabled(fun() ->
-        ?assertNot(
-            maps:is_key(
-                ?CUSTOM_STATUS_EXPIRY_TIMER,
-                refresh_custom_status_expiry_timer(#{custom_status => null})
-            )
-        ),
-        ?assertNot(
-            maps:is_key(
-                ?CUSTOM_STATUS_EXPIRY_TIMER,
-                refresh_custom_status_expiry_timer(#{
-                    custom_status => #{<<"text">> => <<"hi">>}
-                })
-            )
+    ?assertNot(
+        maps:is_key(
+            ?CUSTOM_STATUS_EXPIRY_TIMER,
+            refresh_custom_status_expiry_timer(#{custom_status => null})
         )
-    end).
+    ),
+    ?assertNot(
+        maps:is_key(
+            ?CUSTOM_STATUS_EXPIRY_TIMER,
+            refresh_custom_status_expiry_timer(#{
+                custom_status => #{<<"text">> => <<"hi">>}
+            })
+        )
+    ).
 
 expiry_timer_armed_for_a_future_expiry_test() ->
-    with_expiry_enabled(fun() ->
-        State = refresh_custom_status_expiry_timer(#{custom_status => future_custom_status()}),
-        ?assert(is_reference(maps:get(?CUSTOM_STATUS_EXPIRY_TIMER, State))),
-        ok = cancel_expiry_timer(maps:get(?CUSTOM_STATUS_EXPIRY_TIMER, State))
-    end).
+    State = refresh_custom_status_expiry_timer(#{custom_status => future_custom_status()}),
+    ?assert(is_reference(maps:get(?CUSTOM_STATUS_EXPIRY_TIMER, State))),
+    ok = cancel_expiry_timer(maps:get(?CUSTOM_STATUS_EXPIRY_TIMER, State)).
 
 expiry_timer_not_armed_for_an_already_expired_status_test() ->
-    with_expiry_enabled(fun() ->
-        Expired = #{<<"expires_at">> => <<"2020-01-01T00:00:00.000Z">>},
-        State = refresh_custom_status_expiry_timer(#{custom_status => Expired}),
-        ?assertNot(maps:is_key(?CUSTOM_STATUS_EXPIRY_TIMER, State))
-    end).
+    Expired = #{<<"expires_at">> => <<"2020-01-01T00:00:00.000Z">>},
+    State = refresh_custom_status_expiry_timer(#{custom_status => Expired}),
+    ?assertNot(maps:is_key(?CUSTOM_STATUS_EXPIRY_TIMER, State)).
 
 expiry_timer_replaces_the_previous_one_test() ->
-    with_expiry_enabled(fun() ->
-        First = refresh_custom_status_expiry_timer(#{custom_status => future_custom_status()}),
-        Second = refresh_custom_status_expiry_timer(First),
-        ?assertNotEqual(
-            maps:get(?CUSTOM_STATUS_EXPIRY_TIMER, First),
-            maps:get(?CUSTOM_STATUS_EXPIRY_TIMER, Second)
-        ),
-        ok = cancel_expiry_timer(maps:get(?CUSTOM_STATUS_EXPIRY_TIMER, Second))
-    end).
+    First = refresh_custom_status_expiry_timer(#{custom_status => future_custom_status()}),
+    Second = refresh_custom_status_expiry_timer(First),
+    ?assertNotEqual(
+        maps:get(?CUSTOM_STATUS_EXPIRY_TIMER, First),
+        maps:get(?CUSTOM_STATUS_EXPIRY_TIMER, Second)
+    ),
+    ok = cancel_expiry_timer(maps:get(?CUSTOM_STATUS_EXPIRY_TIMER, Second)).
 
 expiry_timer_is_dropped_when_the_status_is_cleared_test() ->
-    with_expiry_enabled(fun() ->
-        Armed = refresh_custom_status_expiry_timer(#{custom_status => future_custom_status()}),
-        Cleared = refresh_custom_status_expiry_timer(Armed#{custom_status => null}),
-        ?assertNot(maps:is_key(?CUSTOM_STATUS_EXPIRY_TIMER, Cleared))
-    end).
+    Armed = refresh_custom_status_expiry_timer(#{custom_status => future_custom_status()}),
+    Cleared = refresh_custom_status_expiry_timer(Armed#{custom_status => null}),
+    ?assertNot(maps:is_key(?CUSTOM_STATUS_EXPIRY_TIMER, Cleared)).
 
 expiry_timer_delivers_a_reconcile_cast_test() ->
     State = arm_expiry_timer({ok, 1}, #{}),
@@ -343,19 +328,4 @@ future_custom_status() ->
     ),
     #{<<"text">> => <<"brb">>, <<"expires_at">> => list_to_binary(ExpiresAt)}.
 
-with_expiry_enabled(Fun) ->
-    Key = custom_status_expiry_enabled,
-    Previous = application:get_env(fluxer_gateway, Key),
-    application:set_env(fluxer_gateway, Key, true),
-    try
-        Fun()
-    after
-        restore_expiry_env(Key, Previous)
-    end.
-
-restore_expiry_env(Key, undefined) ->
-    application:unset_env(fluxer_gateway, Key);
-restore_expiry_env(Key, {ok, Value}) ->
-    application:set_env(fluxer_gateway, Key, Value).
-
 -endif.

fluxer_gateway/src/presence/presence_payload.erl

@@ -96,46 +96,30 @@ build_invisible_binary_normalized_to_offline_test() ->
     ?assertEqual(null, maps:get(<<"custom_status">>, Result)).
 
 build_clears_expired_custom_status_test() ->
-    with_expiry_enabled(fun() ->
-        Result = build(test_user(), online, false, false, expired_custom_status()),
-        ?assertEqual(<<"online">>, maps:get(<<"status">>, Result)),
-        ?assertEqual(null, maps:get(<<"custom_status">>, Result))
-    end).
+    Result = build(test_user(), online, false, false, expired_custom_status()),
+    ?assertEqual(<<"online">>, maps:get(<<"status">>, Result)),
+    ?assertEqual(null, maps:get(<<"custom_status">>, Result)).
 
 build_coalesces_an_expired_status_with_no_status_test() ->
-    with_expiry_enabled(fun() ->
-        ?assertEqual(
-            build(test_user(), online, false, false, null),
-            build(test_user(), online, false, false, expired_custom_status())
-        )
-    end).
+    ?assertEqual(
+        build(test_user(), online, false, false, null),
+        build(test_user(), online, false, false, expired_custom_status())
+    ).
 
 build_keeps_unexpired_custom_status_test() ->
-    with_expiry_enabled(fun() ->
-        Live = future_custom_status(),
-        Result = build(test_user(), dnd, false, false, Live),
-        ?assertEqual(Live, maps:get(<<"custom_status">>, Result))
-    end).
+    Live = future_custom_status(),
+    Result = build(test_user(), dnd, false, false, Live),
+    ?assertEqual(Live, maps:get(<<"custom_status">>, Result)).
 
 build_keeps_custom_status_without_expires_at_test() ->
-    with_expiry_enabled(fun() ->
-        Live = #{<<"text">> => <<"forever">>},
-        Result = build(test_user(), idle, false, false, Live),
-        ?assertEqual(Live, maps:get(<<"custom_status">>, Result))
-    end).
+    Live = #{<<"text">> => <<"forever">>},
+    Result = build(test_user(), idle, false, false, Live),
+    ?assertEqual(Live, maps:get(<<"custom_status">>, Result)).
 
 build_keeps_malformed_expires_at_test() ->
-    with_expiry_enabled(fun() ->
-        Live = #{<<"text">> => <<"hi">>, <<"expires_at">> => <<"not-a-date">>},
-        Result = build(test_user(), online, false, false, Live),
-        ?assertEqual(Live, maps:get(<<"custom_status">>, Result))
-    end).
-
-build_leaves_expired_status_alone_while_disabled_test() ->
-    application:unset_env(fluxer_gateway, custom_status_expiry_enabled),
-    Expired = expired_custom_status(),
-    Result = build(test_user(), online, false, false, Expired),
-    ?assertEqual(Expired, maps:get(<<"custom_status">>, Result)).
+    Live = #{<<"text">> => <<"hi">>, <<"expires_at">> => <<"not-a-date">>},
+    Result = build(test_user(), online, false, false, Live),
+    ?assertEqual(Live, maps:get(<<"custom_status">>, Result)).
 
 test_user() ->
     #{<<"id">> => <<"1">>, <<"username">> => <<"Test">>}.
@@ -153,19 +137,4 @@ future_custom_status() ->
         erlang:system_time(millisecond) + 3600000, [{unit, millisecond}, {offset, "Z"}]
     ),
     #{<<"text">> => <<"brb">>, <<"expires_at">> => list_to_binary(ExpiresAt)}.
-
-with_expiry_enabled(Fun) ->
-    Key = custom_status_expiry_enabled,
-    Previous = application:get_env(fluxer_gateway, Key),
-    application:set_env(fluxer_gateway, Key, true),
-    try
-        Fun()
-    after
-        restore_expiry_env(Key, Previous)
-    end.
-
-restore_expiry_env(Key, undefined) ->
-    application:unset_env(fluxer_gateway, Key);
-restore_expiry_env(Key, {ok, Value}) ->
-    application:set_env(fluxer_gateway, Key, Value).
 -endif.

fluxer_gateway/src/utils/custom_status_expiry.erl

@@ -4,7 +4,6 @@
 -typing([eqwalizer]).
 
 -export([
-    enabled/0,
     clear_if_expired/1,
     clear_if_expired/2,
     remaining_ms/2,
@@ -20,26 +19,13 @@
 -type wakeup() :: {ok, pos_integer()} | none.
 -type offset() :: {ok, integer()} | none.
 
--define(ENABLED_KEY, custom_status_expiry_enabled).
 -define(MAX_REPAIR_WINDOW_SECONDS, 86400).
 -define(MAX_TIMER_MS, 86400000).
 -define(WAKEUP_JITTER_MS, 5000).
 
--spec enabled() -> boolean().
-enabled() ->
-    application:get_env(fluxer_gateway, ?ENABLED_KEY, false) =:= true.
-
 -spec clear_if_expired(term()) -> custom_status().
 clear_if_expired(CustomStatus) ->
-    clear_when_enabled(enabled(), CustomStatus).
-
--spec clear_when_enabled(boolean(), term()) -> custom_status().
-clear_when_enabled(true, CustomStatus) ->
-    clear_if_expired(CustomStatus, erlang:system_time(millisecond));
-clear_when_enabled(false, CustomStatus) when is_map(CustomStatus) ->
-    CustomStatus;
-clear_when_enabled(false, _CustomStatus) ->
-    null.
+    clear_if_expired(CustomStatus, erlang:system_time(millisecond)).
 
 -spec clear_if_expired(term(), term()) -> custom_status().
 clear_if_expired(CustomStatus, NowMs) when is_map(CustomStatus), is_integer(NowMs) ->
@@ -88,12 +74,6 @@ parse_rfc3339(Chars) ->
 
 -spec next_wakeup_ms(term()) -> wakeup().
 next_wakeup_ms(CustomStatus) ->
-    next_wakeup_ms(enabled(), CustomStatus).
-
--spec next_wakeup_ms(boolean(), term()) -> wakeup().
-next_wakeup_ms(false, _CustomStatus) ->
-    none;
-next_wakeup_ms(true, CustomStatus) ->
     wakeup_delay(remaining_ms(CustomStatus, erlang:system_time(millisecond))).
 
 -spec wakeup_delay(offset()) -> wakeup().
@@ -125,8 +105,7 @@ repair(WindowSeconds) when
     #{
         scheduled => Total,
         considered => length(Considered),
-        window_seconds => WindowSeconds,
-        enabled => enabled()
+        window_seconds => WindowSeconds
     };
 repair(_WindowSeconds) ->
     #{error => invalid_window_seconds}.
@@ -334,21 +313,11 @@ wakeup_clamps_to_the_timer_limit_test() ->
     ?assert(DelayMs > ?MAX_TIMER_MS),
     ?assert(DelayMs =< ?MAX_TIMER_MS + ?WAKEUP_JITTER_MS).
 
-disabled_by_default_test() ->
-    application:unset_env(fluxer_gateway, ?ENABLED_KEY),
-    Status = status(?LIVE_EXPIRES_AT),
-    ?assertEqual(false, enabled()),
-    ?assertEqual(Status, clear_if_expired(Status)),
-    ?assertEqual(none, next_wakeup_ms(future_status())).
-
-enabled_clears_expired_and_arms_future_test() ->
+clears_expired_and_arms_future_test() ->
     Live = future_status(),
-    with_expiry_enabled(fun() ->
-        ?assertEqual(true, enabled()),
-        ?assertEqual(null, clear_if_expired(status(?LIVE_EXPIRES_AT))),
-        ?assertEqual(Live, clear_if_expired(Live)),
-        ?assertMatch({ok, _}, next_wakeup_ms(Live))
-    end).
+    ?assertEqual(null, clear_if_expired(status(?LIVE_EXPIRES_AT))),
+    ?assertEqual(Live, clear_if_expired(Live)),
+    ?assertMatch({ok, _}, next_wakeup_ms(Live)).
 
 future_status() ->
     status(rfc3339(erlang:system_time(millisecond) + 3600000)).
@@ -371,8 +340,7 @@ repair_reports_what_it_scheduled_test() ->
     Result = repair(0),
     ?assert(is_integer(maps:get(scheduled, Result))),
     ?assert(maps:get(scheduled, Result) >= 0),
-    ?assertEqual(0, maps:get(window_seconds, Result)),
-    ?assertEqual(enabled(), maps:get(enabled, Result)).
+    ?assertEqual(0, maps:get(window_seconds, Result)).
 
 schedule_reconciles_delivers_the_cast_test() ->
     flush_mailbox(),
@@ -388,18 +356,4 @@ flush_mailbox() ->
     after 0 -> ok
     end.
 
-with_expiry_enabled(Fun) ->
-    Previous = application:get_env(fluxer_gateway, ?ENABLED_KEY),
-    application:set_env(fluxer_gateway, ?ENABLED_KEY, true),
-    try
-        Fun()
-    after
-        restore_expiry_env(Previous)
-    end.
-
-restore_expiry_env(undefined) ->
-    application:unset_env(fluxer_gateway, ?ENABLED_KEY);
-restore_expiry_env({ok, Value}) ->
-    application:set_env(fluxer_gateway, ?ENABLED_KEY, Value).
-
 -endif.