refactor(voice): remove the unused voice state ack (#2716)

This commit is contained in:
Hampus
2026-09-12 01:37:45 +02:00
committed by GitHub
parent 3e79530389
commit e1bc6c2f7e
16 changed files with 21 additions and 581 deletions
@@ -140,6 +140,4 @@ apply_same_channel_update(Update, ParsedViewerKey) ->
needs_token => false,
viewer_stream_keys => ParsedViewerKey
}),
guild_voice_connection_util:applied_mutation_reply(
UpdateResult, Context, ChannelIdValue
).
UpdateResult.
@@ -181,28 +181,17 @@ normal_update(
) ->
ConnectionId = maps:get(connection_id, Context),
ExistingVS = maps:get(ConnectionId, VoiceStates, #{}),
CurrentVersion = voice_state_utils:voice_state_version(ExistingVS),
MutationDecision = guild_voice_mutation:evaluate(
maps:get(base_version, Context, undefined), CurrentVersion, valid
),
case MutationDecision of
{reject, Reason} ->
guild_voice_connection_util:rejected_mutation_reply(
Context, ExistingVS, State, ChannelIdValue, <<"rejected">>, Reason
);
apply ->
check_permissions(
Context,
ChannelIdValue,
Member,
Channel,
VoiceStates,
State,
IsChannelChange,
ViewerKeyResult,
ExistingVS
)
end.
check_permissions(
Context,
ChannelIdValue,
Member,
Channel,
VoiceStates,
State,
IsChannelChange,
ViewerKeyResult,
ExistingVS
).
-spec check_permissions(
context(),
@@ -19,9 +19,7 @@
maybe_attach_e2ee_key_to_reply/2,
normalize_session_id/1,
normalize_optional_binary/1,
applied_mutation_reply/3,
maybe_error_reply/5,
rejected_mutation_reply/6,
clear_virtual_access_flags/2
]).
@@ -62,9 +60,6 @@ build_context(Request0) ->
viewer_stream_keys => maps:get(viewer_stream_keys, Request, undefined),
latitude => Coord(maps:get(latitude, Request, undefined)),
longitude => Coord(maps:get(longitude, Request, undefined)),
mutation_id => maps:get(mutation_id, Request, undefined),
runtime_epoch => maps:get(runtime_epoch, Request, undefined),
base_version => maps:get(base_version, Request, undefined),
e2ee_capable => Norm(maps:get(e2ee_capable, Request, false)),
bot => Norm(maps:get(bot, Request, false))
}.
@@ -166,72 +161,10 @@ normalize_session_id(Value) ->
normalize_optional_binary(Value) ->
guild_voice_connection_normalize:normalize_optional_binary(Value).
-spec applied_mutation_reply({reply, map(), guild_state()}, context(), integer()) ->
{reply, map(), guild_state()}.
applied_mutation_reply({reply, BaseReply, NewState}, Context, ChannelIdValue) ->
GuildId = guild_voice_connection_normalize:normalize_positive_snowflake(
maps:get(id, NewState, undefined)
),
NewVoiceState = maps:get(voice_state, BaseReply, #{}),
NewVersion = voice_state_utils:voice_state_version(NewVoiceState),
NormalizedConnId = normalize_conn_id_for_ack(Context),
Ack = guild_voice_mutation:build_ack(
maps:get(mutation_id, Context, undefined),
maps:get(runtime_epoch, Context, undefined),
NormalizedConnId,
GuildId,
ChannelIdValue,
#{
status => <<"applied">>,
server_version => NewVersion,
canonical_state => voice_state_utils:external_voice_state(NewVoiceState)
}
),
Reply = merge_ack(BaseReply, Ack),
{reply, Reply, NewState}.
-spec maybe_error_reply(context(), voice_state(), guild_state(), integer(), atom()) ->
{reply, map(), guild_state()} | {reply, {error, atom(), atom()}, guild_state()}.
maybe_error_reply(Context, ExistingVoiceState, State, ChannelIdValue, ErrorAtom) ->
case maps:get(mutation_id, Context, undefined) of
undefined ->
{reply, gateway_errors:error(ErrorAtom), State};
_ ->
rejected_mutation_reply(
Context, ExistingVoiceState, State, ChannelIdValue, <<"rejected">>, ErrorAtom
)
end.
-spec rejected_mutation_reply(
context(), voice_state(), guild_state(), integer(), binary(), atom() | binary()
) -> {reply, map(), guild_state()}.
rejected_mutation_reply(Context, ExistingVS, State, ChannelIdValue, Status, Error) ->
GuildId = guild_voice_connection_normalize:normalize_positive_snowflake(
maps:get(id, State, undefined)
),
CurrentVersion = voice_state_utils:voice_state_version(ExistingVS),
NormalizedConnId = normalize_conn_id_for_ack(Context),
Ack = guild_voice_mutation:build_ack(
maps:get(mutation_id, Context, undefined),
maps:get(runtime_epoch, Context, undefined),
NormalizedConnId,
GuildId,
ChannelIdValue,
#{
status => Status,
server_version => CurrentVersion,
canonical_state => canonical_state_for_ack(ExistingVS),
error_code => rejection_error_code(Error),
error_message => rejection_error_message(Error)
}
),
{reply, #{success => false, ack => Ack}, State}.
-spec canonical_state_for_ack(voice_state()) -> map().
canonical_state_for_ack(VoiceState) when is_map(VoiceState), map_size(VoiceState) > 0 ->
voice_state_utils:external_voice_state(VoiceState);
canonical_state_for_ack(_) ->
#{}.
maybe_error_reply(_Context, _ExistingVoiceState, State, _ChannelIdValue, ErrorAtom) ->
{reply, gateway_errors:error(ErrorAtom), State}.
-spec clear_virtual_access_flags(voice_state(), guild_state()) -> guild_state().
clear_virtual_access_flags(VoiceState, State) when is_map(VoiceState) ->
@@ -244,26 +177,6 @@ clear_virtual_access_flags(VoiceState, State) when is_map(VoiceState) ->
State
end.
-spec normalize_conn_id_for_ack(context()) -> binary() | null.
normalize_conn_id_for_ack(Context) ->
case maps:get(connection_id, Context, undefined) of
undefined -> null;
C -> C
end.
-spec merge_ack(map(), term()) -> map().
merge_ack(BaseReply, undefined) -> BaseReply;
merge_ack(BaseReply, Ack) -> BaseReply#{ack => Ack}.
-spec rejection_error_code(atom() | binary()) -> binary().
rejection_error_code(ErrorAtom) when is_atom(ErrorAtom) -> gateway_errors:error_code(ErrorAtom);
rejection_error_code(ErrorCode) when is_binary(ErrorCode) -> ErrorCode.
-spec rejection_error_message(atom() | binary()) -> binary().
rejection_error_message(ErrorAtom) when is_atom(ErrorAtom) ->
gateway_errors:error_message(ErrorAtom);
rejection_error_message(ErrorMessage) when is_binary(ErrorMessage) -> ErrorMessage.
-spec guild_data(guild_state()) -> map().
guild_data(State) ->
map_utils:ensure_map(maps:get(data, State, #{})).
@@ -1,129 +0,0 @@
%% SPDX-License-Identifier: AGPL-3.0-or-later
-module(guild_voice_mutation).
-typing([eqwalizer]).
-export([evaluate/3, build_ack/6]).
-ifdef(TEST).
-include_lib("eunit/include/eunit.hrl").
-endif.
-spec evaluate(
BaseVersion :: integer() | undefined,
CurrentVersion :: integer(),
ValidationResult :: valid | invalid
) -> apply | {reject, binary()}.
evaluate(_BaseVersion, _CurrentVersion, invalid) ->
{reject, <<"invalid_payload">>};
evaluate(BaseVersion, CurrentVersion, valid) when
is_integer(BaseVersion), BaseVersion < CurrentVersion - 1
->
{reject, <<"stale_base_version">>};
evaluate(_BaseVersion, _CurrentVersion, valid) ->
apply.
-spec build_ack(
MutationId :: binary() | undefined,
RuntimeEpoch :: binary() | undefined,
ConnectionId :: binary() | null,
GuildId :: integer() | null | undefined,
ChannelId :: integer() | null | undefined,
Outcome :: #{
status := binary(),
server_version := integer(),
canonical_state := map(),
error_code => binary(),
error_message => binary()
}
) -> map() | undefined.
build_ack(undefined, _RuntimeEpoch, _ConnectionId, _GuildId, _ChannelId, _Outcome) ->
undefined;
build_ack(MutationId, RuntimeEpoch, ConnectionId, GuildId, ChannelId, Outcome) when
is_binary(MutationId)
->
BaseAck = #{
<<"mutation_id">> => MutationId,
<<"runtime_epoch">> => RuntimeEpoch,
<<"connection_id">> => ConnectionId,
<<"guild_id">> => maybe_int_to_binary(GuildId),
<<"channel_id">> => maybe_int_to_binary(ChannelId),
<<"status">> => maps:get(status, Outcome),
<<"server_version">> => maps:get(server_version, Outcome),
<<"canonical_state">> => maps:get(canonical_state, Outcome)
},
put_optional_ack_fields(BaseAck, Outcome).
-spec maybe_int_to_binary(integer() | null | undefined) -> binary() | null.
maybe_int_to_binary(null) -> null;
maybe_int_to_binary(undefined) -> null;
maybe_int_to_binary(N) when is_integer(N) -> integer_to_binary(N).
-spec maybe_put_optional_ack_field(map(), binary(), binary() | undefined) -> map().
maybe_put_optional_ack_field(Ack, _Field, undefined) ->
Ack;
maybe_put_optional_ack_field(Ack, Field, Value) ->
Ack#{Field => Value}.
-spec put_optional_ack_fields(map(), map()) -> map().
put_optional_ack_fields(BaseAck, Outcome) ->
lists:foldl(
fun({Field, Key}, Ack) ->
maybe_put_optional_ack_field(Ack, Field, maps:get(Key, Outcome, undefined))
end,
BaseAck,
[
{<<"error_code">>, error_code},
{<<"error_message">>, error_message}
]
).
-ifdef(TEST).
evaluate_invalid_payload_test() ->
?assertEqual({reject, <<"invalid_payload">>}, evaluate(0, 0, invalid)),
?assertEqual({reject, <<"invalid_payload">>}, evaluate(undefined, 5, invalid)).
evaluate_apply_no_version_test() ->
?assertEqual(apply, evaluate(undefined, 0, valid)),
?assertEqual(apply, evaluate(undefined, 100, valid)).
evaluate_apply_current_version_test() ->
?assertEqual(apply, evaluate(5, 5, valid)).
evaluate_apply_one_behind_test() ->
?assertEqual(apply, evaluate(4, 5, valid)).
evaluate_stale_base_test() ->
?assertEqual({reject, <<"stale_base_version">>}, evaluate(3, 5, valid)),
?assertEqual({reject, <<"stale_base_version">>}, evaluate(0, 10, valid)).
build_ack_no_mutation_id_test() ->
Outcome = #{status => <<"ignored">>, server_version => 0, canonical_state => #{}},
?assertEqual(undefined, build_ack(undefined, undefined, <<"conn1">>, 123, 456, Outcome)).
test_build_ack(Outcome) ->
build_ack(<<"mut1">>, <<"epoch1">>, <<"conn1">>, 123, 456, Outcome).
build_ack_full_test() ->
Outcome = #{status => <<"applied">>, server_version => 7, canonical_state => #{}},
#{
<<"mutation_id">> := <<"mut1">>,
<<"status">> := <<"applied">>,
<<"server_version">> := 7
} = test_build_ack(Outcome).
build_ack_with_error_fields_test() ->
Outcome = #{
status => <<"rejected">>,
server_version => 9,
canonical_state => #{},
error_code => <<"stale_base_version">>,
error_message => <<"stale_base_version">>
},
#{
<<"error_code">> := <<"stale_base_version">>,
<<"error_message">> := <<"stale_base_version">>
} = test_build_ack(Outcome).
-endif.
@@ -53,12 +53,6 @@ handle_voice_state_update(Data, State) ->
-spec extract_voice_params(map()) -> map().
extract_voice_params(Data) ->
BaseVersionRaw = maps:get(<<"base_version">>, Data, undefined),
BaseVersion =
case BaseVersionRaw of
V when is_integer(V), V >= 0 -> V;
_ -> undefined
end,
#{
guild_id_raw => maps:get(<<"guild_id">>, Data, null),
channel_id_raw => maps:get(<<"channel_id">>, Data, null),
@@ -70,10 +64,7 @@ extract_voice_params(Data) ->
viewer_stream_keys => maps:get(<<"viewer_stream_keys">>, Data, undefined),
is_mobile => maps:get(<<"is_mobile">>, Data, false),
latitude => maps:get(<<"latitude">>, Data, null),
longitude => maps:get(<<"longitude">>, Data, null),
mutation_id => maps:get(<<"mutation_id">>, Data, undefined),
runtime_epoch => maps:get(<<"runtime_epoch">>, Data, undefined),
base_version => BaseVersion
longitude => maps:get(<<"longitude">>, Data, null)
}.
-spec dispatch_validated(
@@ -510,9 +501,6 @@ build_guild_request(ChId, Params, UserId, SId, E2EE, Bot) ->
is_mobile => maps:get(is_mobile, Params),
latitude => maps:get(latitude, Params),
longitude => maps:get(longitude, Params),
mutation_id => maps:get(mutation_id, Params),
runtime_epoch => maps:get(runtime_epoch, Params),
base_version => maps:get(base_version, Params),
e2ee_capable => E2EE,
bot => Bot
}.
@@ -102,14 +102,6 @@ handle_guild_reply_ok(Reply, Ctx, SessionPid) ->
maybe_dispatch_voice_server_update(
Reply, GId, ChId, SessionPid
),
case maps:get(ack, Reply, undefined) of
Ack when is_map(Ack) ->
dispatch_to_session(
SessionPid, voice_state_ack, Ack, GId
);
_ ->
ok
end,
ok.
-spec maybe_dispatch_voice_server_update(
@@ -305,9 +297,9 @@ dispatch_to_session_converts_wire_payload_test() ->
<<"permissions">> => 8,
<<"roles">> => [456]
},
ok = dispatch_to_session(self(), voice_state_ack, Payload, 123),
ok = dispatch_to_session(self(), voice_state_update, Payload, 123),
receive
{'$gen_cast', {dispatch, voice_state_ack, WirePayload}} ->
{'$gen_cast', {dispatch, voice_state_update, WirePayload}} ->
?assertEqual(
#{
<<"id">> => <<"123">>,
@@ -362,26 +354,6 @@ plain_success_reply_does_not_dispatch_test() ->
ok = handle_guild_reply_ok(#{success => true}, test_voice_ctx(456), self()),
assert_no_dispatch().
rejected_mutation_reply_dispatches_ack_only_test() ->
Ack = #{<<"status">> => <<"rejected">>, <<"mutation_id">> => <<"m1">>},
Reply = #{success => false, ack => Ack},
ok = handle_guild_reply_ok(Reply, test_voice_ctx(456), self()),
Payload = receive_dispatch(voice_state_ack),
?assertEqual(<<"rejected">>, maps:get(<<"status">>, Payload)),
assert_no_dispatch().
in_channel_update_reply_dispatches_ack_test() ->
Ack = #{<<"status">> => <<"applied">>, <<"mutation_id">> => <<"m2">>},
Reply = #{
success => true,
voice_state => #{<<"channel_id">> => <<"456">>},
ack => Ack
},
ok = handle_guild_reply_ok(Reply, test_voice_ctx(456), self()),
Payload = receive_dispatch(voice_state_ack),
?assertEqual(<<"applied">>, maps:get(<<"status">>, Payload)),
assert_no_dispatch().
join_reply_dispatches_voice_server_update_test() ->
Reply = #{
success => true,
@@ -91,22 +91,6 @@ voice_state_update_connection_not_found_test() ->
Request, State, {error, not_found, voice_connection_not_found}
).
voice_state_update_connection_not_found_returns_rejected_ack_test() ->
State = base_test_state(),
Request = #{
user_id => 10,
channel_id => 100,
connection_id => <<"missing-conn">>,
mutation_id => <<"m-missing-connection">>,
runtime_epoch => <<"epoch-1">>,
base_version => 0
},
Ack = rejected_voice_state_ack(Request, State),
?assertEqual(<<"rejected">>, maps:get(<<"status">>, Ack)),
?assertEqual(0, maps:get(<<"server_version">>, Ack)),
?assertEqual(#{}, maps:get(<<"canonical_state">>, Ack)),
?assertEqual(<<"VOICE_CONNECTION_NOT_FOUND">>, maps:get(<<"error_code">>, Ack)).
voice_state_update_invalid_viewer_stream_keys_test() ->
State = connected_state(<<"10">>),
Request = #{
@@ -147,98 +131,6 @@ voice_state_update_rejects_dm_scope_viewer_stream_key_test() ->
Request, State, {error, validation_error, voice_invalid_state}
).
voice_state_update_viewer_stream_keys_missing_connection_returns_rejected_ack_test() ->
VoiceStates = #{
<<"conn-1">> => #{
<<"channel_id">> => <<"100">>,
<<"connection_id">> => <<"conn-1">>,
<<"user_id">> => <<"10">>,
<<"version">> => 2
}
},
State = maps:put(voice_states, VoiceStates, base_test_state()),
Request = #{
user_id => 10,
channel_id => 100,
connection_id => <<"conn-1">>,
mutation_id => <<"m-missing-watched">>,
runtime_epoch => <<"epoch-1">>,
base_version => 2,
viewer_stream_keys => [<<"999:100:missing-conn">>]
},
Ack = rejected_voice_state_ack(Request, State),
?assertEqual(<<"rejected">>, maps:get(<<"status">>, Ack)),
?assertEqual(<<"VOICE_CONNECTION_NOT_FOUND">>, maps:get(<<"error_code">>, Ack)).
voice_state_update_stale_base_version_returns_rejected_ack_test() ->
VoiceStates = #{
<<"conn-1">> => #{
<<"channel_id">> => <<"100">>,
<<"connection_id">> => <<"conn-1">>,
<<"user_id">> => <<"10">>,
<<"version">> => 5
}
},
State = maps:put(voice_states, VoiceStates, base_test_state()),
Request = #{
user_id => 10,
channel_id => 100,
connection_id => <<"conn-1">>,
mutation_id => <<"m1">>,
runtime_epoch => <<"epoch-1">>,
base_version => 3
},
Ack = rejected_voice_state_ack(Request, State),
?assertEqual(<<"rejected">>, maps:get(<<"status">>, Ack)),
?assertEqual(5, maps:get(<<"server_version">>, Ack)),
?assertEqual(<<"stale_base_version">>, maps:get(<<"error_code">>, Ack)).
voice_state_update_stale_base_version_no_superseded_arm_test() ->
VoiceStates = #{
<<"conn-2">> => #{
<<"channel_id">> => <<"100">>,
<<"connection_id">> => <<"conn-2">>,
<<"user_id">> => <<"10">>,
<<"version">> => 10
}
},
State = maps:put(voice_states, VoiceStates, base_test_state()),
Request = #{
user_id => 10,
channel_id => 100,
connection_id => <<"conn-2">>,
mutation_id => <<"m-reg">>,
runtime_epoch => <<"epoch-reg">>,
base_version => 7
},
Ack = rejected_voice_state_ack(Request, State),
?assertEqual(<<"rejected">>, maps:get(<<"status">>, Ack)),
?assertEqual(10, maps:get(<<"server_version">>, Ack)),
?assertEqual(<<"stale_base_version">>, maps:get(<<"error_code">>, Ack)).
voice_state_update_invalid_viewer_stream_keys_returns_rejected_ack_test() ->
VoiceStates = #{
<<"conn-1">> => #{
<<"channel_id">> => <<"100">>,
<<"connection_id">> => <<"conn-1">>,
<<"user_id">> => <<"10">>,
<<"version">> => 2
}
},
State = maps:put(voice_states, VoiceStates, base_test_state()),
Request = #{
user_id => 10,
channel_id => 100,
connection_id => <<"conn-1">>,
mutation_id => <<"m2">>,
runtime_epoch => <<"epoch-1">>,
base_version => 2,
viewer_stream_keys => 123
},
Ack = rejected_voice_state_ack(Request, State),
?assertEqual(<<"rejected">>, maps:get(<<"status">>, Ack)),
?assertEqual(<<"VOICE_INVALID_STATE">>, maps:get(<<"error_code">>, Ack)).
voice_state_update_guild_id_missing_test() ->
State0 = base_test_state(),
State1 = replace_guild_id(State0, undefined),
@@ -487,9 +379,3 @@ viewer_keys(State) ->
assert_voice_state_update_error(Request, State, Error) ->
{reply, Error, _} = guild_voice_connection:voice_state_update(Request, State).
-spec rejected_voice_state_ack(map(), map()) -> map().
rejected_voice_state_ack(Request, State) ->
{reply, #{ack := #{} = Ack, success := false}, _} =
guild_voice_connection:voice_state_update(Request, State),
Ack.