From cdf52da96649884fd4b0d0933976a4ad99b3c48f Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Tue, 2 Jun 2020 10:39:27 +0200 Subject: [PATCH 01/15] rollback sys_bpe or clientId? --- apps/roster/src/protocol/roster_message.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 99119a529..815cff2ea 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -486,7 +486,7 @@ proc({#io{} = IO, ClientId}, #handler{state = C} = H) -> proc({#'Message'{status = update} = Msg, ClientId}, #handler{state = C} = H) -> ?LOG_INFO("UPDATE TRANSCRIBE: ~p", [Msg]), %% As a dirty hack, we let sys_bpe post the messaage update. The client itself may have disconnected - try {reply, {bert, IO}, _, _} = info(Msg, {[], handled}, #cx{params = <<"sys_bpe">>, client_pid = C, state = ack}), + try {reply, {bert, IO}, _, _} = info(Msg, {[], handled}, #cx{params = ClientId, client_pid = C, state = ack}), roster:send_action(C, ClientId, IO) catch Err:Rea:Sta -> ?LOG_ERROR("~p:~p:~p", [Err, Rea, Sta]) -- GitLab From 12cdc694146a58ae460ac4ff77d98ad16e036f03 Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Tue, 2 Jun 2020 10:36:33 +0200 Subject: [PATCH 02/15] Improved test suite Add untranscribe to tests Add translate tests untranslate tests --- test/nynja.erl | 62 ++++++++++++++ test/transcribe_SUITE.erl | 170 +++++++++++++++++++++++++++++++++++--- 2 files changed, 219 insertions(+), 13 deletions(-) diff --git a/test/nynja.erl b/test/nynja.erl index 6b5289235..9f7ce8f6c 100644 --- a/test/nynja.erl +++ b/test/nynja.erl @@ -262,11 +262,28 @@ transcribe_msg_room(User = #user{ client = ClientId, roster_id = PhoneId}, RoomN N = if Ack -> 2; true -> 1 end, ws_send(User, mqtt_publish(ClientId, MsgS), N, 30000). +translate_msg_room(User = #user{ client = ClientId, roster_id = PhoneId}, RoomName, IdMessage, Lang, Ack) -> + RoomId = typed_uuid(<<"room">>, RoomName), + MsgS = make_translate_message(ClientId, PhoneId, IdMessage, #muc{ name = RoomId }, Lang, Ack), + N = if Ack -> 2; true -> 1 end, + ws_send(User, mqtt_publish(ClientId, MsgS), N, 30000). + +untranslate_msg_room(User = #user{ client = ClientId, roster_id = PhoneId}, RoomName, IdMessage, DescId, false = Ack) -> + RoomId = typed_uuid(<<"room">>, RoomName), + MsgS = make_untranslate_message(ClientId, PhoneId, IdMessage, #muc{ name = RoomId }, DescId), + N = if Ack -> 2; true -> 1 end, + ws_send(User, mqtt_publish(ClientId, MsgS), N, 10000). + transcribe_msg_chat(User = #user{ client = ClientId, roster_id = PhoneId}, FeedId, IdMessage, Type, Ack) -> MsgS = make_transcribe_message(ClientId, PhoneId, IdMessage, FeedId, Type, true), N = if Ack -> 2; true -> 1 end, ws_send(User, mqtt_publish(ClientId, MsgS), N, 30000). +untranscribe_msg_chat(User = #user{ client = ClientId, roster_id = PhoneId}, FeedId, IdMessage, DescId, Ack) -> + MsgS = make_untranscribe_message(ClientId, PhoneId, IdMessage, FeedId, DescId), + N = if Ack -> 2; true -> 1 end, + ws_send(User, mqtt_publish(ClientId, MsgS), N, 10000). + make_room_message(User, RoomName, Msg) -> make_room_message(User, RoomName, Msg, false). @@ -350,6 +367,51 @@ make_transcribe_message(ClientId, PhoneId, IdMessage, Feed, Type, Ack) -> files = PayloadDesc ++ AckDesc, status = update }. +%% NO ack in client... hard to see what Id of ack should be. +make_untranscribe_message(_ClientId, PhoneId, IdMessage, Feed, DescId) -> + %% Lookup USERS feature for this PhoneId in the Features of Msgs and use the ID of + %% that description to put it to [] + PayloadDesc = [ #'Desc'{ id = DescId, mime = [] } ], + #'Message'{ id = IdMessage, + feed_id = Feed, + container = chain, + from = PhoneId, + to = [], + files = PayloadDesc, + status = update }. + +%% This is done client side, we don't really know the format +make_translate_message(ClientId, PhoneId, IdMessage, Feed, Lang, Ack) -> + TS = integer_to_binary(uniq()), + ID = <>, + Data = + [#'Feature'{id = <>, + key = <<"LANGUAGE">>,value = Lang, + group = <<"FILE_DATA">>}, + #'Feature'{id = <>, + key = <<"USERS">>, + value = PhoneId, + group = <<"FILE_DATA">>}, + #'Feature'{id = <>, + key = <<"TRANSLATION">>, + value = iolist_to_binary(["This is the text translated to ", Lang]), + group = <<"FILE_DATA">>}], + PayloadDesc = [ #'Desc'{ id = ID, mime = <<"translate">>, + payload = <<"This is the original text">>, + data = Data } ], + AckDesc = [ #'Desc'{ id = <<"ack", TS/binary>>, mime = <<"ack">> } || Ack ], %% no ack in client + #'Message'{ id = IdMessage, + feed_id = Feed, %% not really needed + container = chain, %% not really needed + from = PhoneId, + to = [], + files = PayloadDesc ++ AckDesc, + status = update }. + +%% NO ack in client... hard to see what Id of ack should be. +make_untranslate_message(ClientId, PhoneId, IdMessage, Feed, DescId) -> + %% same as untranscribe + make_untranscribe_message(ClientId, PhoneId, IdMessage, Feed, DescId). make_member(#{ phone_id := PhoneId } = Args) -> Settings = diff --git a/test/transcribe_SUITE.erl b/test/transcribe_SUITE.erl index 40317adf3..a36e48058 100644 --- a/test/transcribe_SUITE.erl +++ b/test/transcribe_SUITE.erl @@ -1,6 +1,16 @@ %% Test transcription and translation of messages %% Which uses google API +%% This is how it used to work (May 2020, on staging). +%% Both parties can transcribe an audio message, but the result is only +%% visible to them. The other party may possibly receive that message, +%% but the transcribe and untranscribe are not made visible in the web client. +%% This is a client property, to make a transcription visible. +%% +%% This visibility issue cannot be tested without the real clients +%% What happens is the addition of a feature with a USERS key +%% {'Feature', <<"..._users">>, <<"USERS">>, Key,<<"DUMMY_GROUP">>} + -module(transcribe_SUITE). -include_lib("stdlib/include/assert.hrl"). -include_lib("emqttc/include/emqttc_packet.hrl"). @@ -22,10 +32,11 @@ , short_transcribe/1 , long_transcribe/1 , chat_transcribe/1 + , translate/1 ]). all() -> - [ %% {group, translate}, + [{group, translate}, {group, transcribe} ]. @@ -110,9 +121,10 @@ short_transcribe(Cfg) -> ct:log("User notified ~p", [UserNotify]), %% Admin sends audio - [#mqtt_packet{ payload = #'Message'{ id = IdMessage } } | _Ack ] = - nynja:audio_msg_room(AdminUser, RoomName, - <<"https://nynja-defaults.s3.us-west-2.amazonaws.com/58f1b135-2c43-48b6-b83a-129254e567f2.mpeg">>, true), + IdMessage = + ack_and_msgid( + nynja:audio_msg_room(AdminUser, RoomName, + <<"https://nynja-defaults.s3.us-west-2.amazonaws.com/58f1b135-2c43-48b6-b83a-129254e567f2.mpeg">>, true)), ct:log("Id for 1st audio msg ~p", [IdMessage]), _ = nynja:ws_receive(User), %% User gets this msg @@ -127,9 +139,10 @@ short_transcribe(Cfg) -> _ = nynja:ws_receive(User, any, 10), %% User sends audio - [#mqtt_packet{ payload = #'Message'{ id = IdMessage2 } } | _ ] = - nynja:audio_msg_room(User, RoomName, - <<"https://nynja-defaults.s3.us-west-2.amazonaws.com/5b3b3a41-0439-43ab-9bdf-c847e9c72780.mpeg">>, true), + IdMessage2 = + ack_and_msgid( + nynja:audio_msg_room(User, RoomName, + <<"https://nynja-defaults.s3.us-west-2.amazonaws.com/5b3b3a41-0439-43ab-9bdf-c847e9c72780.mpeg">>, true)), ct:log("Id for 2nd audio msg ~p", [IdMessage2]), _ = nynja:ws_receive(AdminUser), %% AdminUser gets this msg @@ -164,9 +177,10 @@ long_transcribe(Cfg) -> ct:log("User notified ~p", [UserNotify]), %% Admin sends audio - [#mqtt_packet{ payload = #'Message'{ id = IdMessage } } | _Ack ] = - nynja:audio_msg_room(AdminUser, RoomName, - <<"https://nynja-defaults.s3.us-west-2.amazonaws.com/58f1b135-2c43-48b6-b83a-129254e567f2.mpeg">>, true), + IdMessage = + ack_and_msgid( + nynja:audio_msg_room(AdminUser, RoomName, + <<"https://nynja-defaults.s3.us-west-2.amazonaws.com/58f1b135-2c43-48b6-b83a-129254e567f2.mpeg">>, true)), ct:log("Id for 1st audio msg ~p", [IdMessage]), _ = nynja:ws_receive(User), %% User gets this msg @@ -203,12 +217,130 @@ chat_transcribe(Cfg) -> nynja:audio_msg_chat(User, Friend, <<"https://nynja-defaults.s3.us-west-2.amazonaws.com/58f1b135-2c43-48b6-b83a-129254e567f2.mpeg">>, FeedId), ct:log("Audio Msg ~p: ~p", [IdMessage, Audio]), + [#mqtt_packet{ payload = #'Message'{ id = FriendIdMessage } }] = nynja:ws_receive(Friend), %% Friend gets this msg + ct:log("Friend audio msg ~p", [FriendIdMessage]), [#mqtt_packet{ payload = {io, {ok, transcribe}, IdMessage} }, #mqtt_packet{ payload = #'Message'{id = IdMessage, status = update, files = Data}}] = nynja:transcribe_msg_chat(User, FeedId, IdMessage, <<"short">>, true), ct:log("Transcribe result = ~p", [Data]), #'Desc'{mime = <<"transcribe">>, payload = <<"1 2 3 4 5">>} = lists:last(Data), + + %% Both parties get the result, but client filters what is shown to them. + FM = nynja:ws_receive(Friend, any, 10), + ct:log("Transcribe result for friend = ~p", [FM]), + UM = nynja:ws_receive(User, any, 10), + ct:log("Transcribe result for user = ~p", [UM]), + + %% Friend sends audio + #mqtt_packet{ payload = #'Message'{} } = + nynja:audio_msg_chat(Friend, User, + <<"https://nynja-defaults.s3.us-west-2.amazonaws.com/5b3b3a41-0439-43ab-9bdf-c847e9c72780.mpeg">>, FeedId), + [#mqtt_packet{ payload = #'Message'{ id = IdMessage2 } }] = nynja:ws_receive(User), %% User gets this msg + ct:log("Id for 2nd audio msg ~p", [IdMessage2]), + + %% Friend disconnects, we still want to be able to transcribe + nynja:ws_close(Friend), + + %% User transcribes Friend's audio + [#mqtt_packet{ payload = {io, {ok, transcribe}, IdMessage2} }, + #mqtt_packet{ payload = #'Message'{id = IdMessage2, status = update, files = UserSeesTranscribe2}}] = + nynja:transcribe_msg_chat(User, FeedId, IdMessage2, <<"short">>, true), + + ct:log("User sees transcribe = ~p", [UserSeesTranscribe2]), + #'Desc'{id = TranscribeId, mime = <<"transcribe">>, payload = <<"this is a test">>} = lists:last(UserSeesTranscribe2), + + %% Now untranscribe this message + #mqtt_packet{ payload = #'Message'{status = update, files = UnTranscribe}} = + nynja:untranscribe_msg_chat(User, FeedId, IdMessage2, TranscribeId, false), + ct:log("User UnTranscribe = ~p", [UnTranscribe]), + ?assertEqual([], [ D || #'Desc'{mime = <<"transcribe">>} = D <- UnTranscribe]), + ok. + +translate(Cfg) -> + Phone = proplists:get_value(phone, Cfg, <<"000000038">>), + MemberPhone1 = <<"001002003">>, + MemberPhone2 = <<"001002004">>, + + + register_user([{phone, Phone}]), %% may fail if already registered + register_user([{phone, MemberPhone1}, {fname, <<"Kalle">>}]), + register_user([{phone, MemberPhone2}, {fname, <<"Lisa">>}]), + + AdminUser = nynja:connect_user(Phone), + User1 = nynja:connect_user(MemberPhone1), + User2 = nynja:connect_user(MemberPhone2), + RoomName = iolist_to_binary(["Translate-", integer_to_list(os:system_time())]), + + ct:log("Creating room ~p", [RoomName]), + nynja:create_room(AdminUser, #{name => RoomName}), + nynja:add_to_room(AdminUser, User1, RoomName, 1), + nynja:add_to_room(AdminUser, User2, RoomName, 1), + + %% cleanup all unreceived messages + nynja:ws_receive(User1, any, 10), + nynja:ws_receive(User2, any, 10), + + #mqtt_packet{ payload = #'Message'{} } = + nynja:msg_room(AdminUser, RoomName, <<"Welcome to our nynja room">>), + [ #mqtt_packet{ payload = #'Message'{id = IdMessage}} = WelcomeMsg] = nynja:ws_receive(User1), %% User1 gets this msg + [ #mqtt_packet{ payload = #'Message'{id = IdMessage}} ] = nynja:ws_receive(User2), %% User2 gets this msg + + ct:log("User sees ~p", [WelcomeMsg]), + + #mqtt_packet{ payload = #'Message'{id = IdMessage, status = update, files = Descs1}} = + nynja:translate_msg_room(User1, RoomName, IdMessage, <<"sv">>, false), + ct:log("User sees one translation ~p in sv", [Descs1]), + [ TRDesc1 ] = [ D || #'Desc'{mime = <<"translate">>} = D <- Descs1], + ?assertEqual(<<"sv">>, get_desc_value(<<"LANGUAGE">>, TRDesc1)), + + #mqtt_packet{ payload = #'Message'{id = IdMessage, status = update, files = Descs2}} = + nynja:translate_msg_room(User1, RoomName, IdMessage, <<"fr">>, false), + ct:log("User only sees second translation ~p in fr", [Descs2]), + [ TRDesc2 ] = [ D || #'Desc'{mime = <<"translate">>} = D <- Descs2], + ?assertEqual(<<"fr">>, get_desc_value(<<"LANGUAGE">>, TRDesc2)), + + _ = nynja:ws_receive(User2, any, 10), %% Second user and Admin have received translate messages + _ = nynja:ws_receive(AdminUser, any, 10), + + #mqtt_packet{ payload = #'Message'{id = IdMessage, status = update, files = Descs3}} = + nynja:translate_msg_room(User2, RoomName, IdMessage, <<"fr">>, false), + ct:log("User2 also reads it in french ~p in fr", [Descs3]), + [ #'Desc'{id = FrenchId} = TRDesc3 ] = [ D || #'Desc'{mime = <<"translate">>} = D <- Descs3], + ?assertEqual(<<"fr">>, get_desc_value(<<"LANGUAGE">>, TRDesc3)), + ?assertEqual(2, length(string:split(get_desc_value(<<"USERS">>, TRDesc3), ",", all))), + + _ = nynja:ws_receive(User1, any, 10), %% User and Admin have received translate messages + _ = nynja:ws_receive(AdminUser, any, 10), + + #mqtt_packet{ payload = #'Message'{id = IdMessage, status = update, files = Descs4}} = + nynja:translate_msg_room(User1, RoomName, IdMessage, <<"sv">>, false), + ct:log("User only sees translation ~p in sv other user sees it in fr", [Descs4]), + [ TRDesc4, TRDesc5 ] = [ D || #'Desc'{mime = <<"translate">>} = D <- Descs4], + Languages = [ get_desc_value(<<"LANGUAGE">>, TRDesc4), get_desc_value(<<"LANGUAGE">>, TRDesc5)], + ?assert(true, lists:member(<<"sv">>, Languages) andalso lists:member(<<"fr">>, Languages)), + + _ = nynja:ws_receive(User2, any, 10), %% Second user and Admin have received translate messages + _ = nynja:ws_receive(AdminUser, any, 10), + + %% Now try untranslate the french message with invalid client + #mqtt_packet{ payload = #'Message'{status = update, files = Descs5}} = + nynja:untranslate_msg_room(AdminUser, RoomName, IdMessage, FrenchId, false), + ct:log("AdminUser UnTranslate = ~p", [Descs5]), + ?assertEqual(3, length(Descs5)), %% welcome message and both translations + + %% Cleanup + _ = nynja:ws_receive(User1, any, 10), + _ = nynja:ws_receive(User2, any, 10), + + %% Now untranslate the french message with valid client + #mqtt_packet{ payload = #'Message'{status = update, files = Descs6}} = + nynja:untranslate_msg_room(User2, RoomName, IdMessage, FrenchId, false), + ct:log("User2 UnTranslate = ~p", [Descs6]), + [ TRDesc6 ] = [ D || #'Desc'{mime = <<"translate">>} = D <- Descs6], + ?assertEqual(<<"sv">>, get_desc_value(<<"LANGUAGE">>, TRDesc6)), + ?assertEqual(1, length(string:split(get_desc_value(<<"USERS">>, TRDesc6), ",", all))), + ok. %%% ------------- helpers --------------------- @@ -220,9 +352,9 @@ request(Id, Params, Config) -> {ok, Status, Headers, Body} = api_nynja:http_request(Request, [{timeout, 10000}], [], - [{logfun, fun(F, As) -> - ct:log(F, As) - end}]), + [{logfun, fun(F, As) -> + ct:log(F, As) + end}]), Response = api_nynja:validate_response(Id, Request, Status, Headers, Body), [ ct:log("Validated ~p\n", [Response]) || true], Response. @@ -234,3 +366,15 @@ basic_auth() -> Password = proplists:get_value(password, BasicAuth), {security, {authorization_basic, [#{username => User, password => Password}]}}. + +ack_and_msgid(Msgs) -> + case Msgs of + [#mqtt_packet{ payload = #'Message'{ id = IdM } }, #mqtt_packet{ payload = #'MessageAck'{} } ] -> IdM; + [#mqtt_packet{ payload = #'MessageAck'{} }, #mqtt_packet{ payload = #'Message'{ id = IdM } } ] -> IdM + end. + +get_desc_value(Key, Desc) -> + case roster:get_data(Key, Desc) of + [] -> undefined; + #'Feature'{value = V} -> V + end. -- GitLab From 30a8eae2faa78912a0b172a23600075e526f3544 Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Tue, 2 Jun 2020 10:37:35 +0200 Subject: [PATCH 03/15] refactor roster_message:info --- apps/roster/src/protocol/roster_message.erl | 74 ++++++++++++--------- 1 file changed, 43 insertions(+), 31 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 815cff2ea..048b0ef66 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -406,7 +406,7 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" end; -info(#'Message'{status = update, id = Id, feed_id = Feed, from = From, +info(#'Message'{status = update, id = Id, from = From, files = [#'Desc'{id = ID, payload = Payload, data = Data, mime = DMime} = ND | _]}, Req, #cx{params = ClientId, client_pid = C, state=ack} = State) when is_integer(Id) -> PhoneId = case ClientId of ?SYS_SCHED_CLIENT -> From; <<"emqttd_", _/binary>> -> roster:phone_id(ClientId) end, @@ -416,42 +416,54 @@ info(#'Message'{status = update, id = Id, feed_id = Feed, from = From, {ok, #'Message'{feed_id = Feed, files = Descs} = Msg} -> case roster:is_visible_msg(Msg, PhoneId) of 1 -> - Descs2 = lists:flatten(lists:foldr( - fun (#'Desc'{data = NData, id = DescId} = D, [Acc, _]) when Lang == [], ID /= [] -> - #'Feature'{value = I} = - case roster:get_data(?USERS_KEY, NData) of - [] -> #'Feature'{key = ?USERS_KEY, value = []}; FU -> FU end, - [[case DescId of - ID -> roster:set_data(#'Feature'{key = ?USERS_KEY, value = binary_delpart(I, PhoneId)}, D); - _-> D end | Acc], []]; - (#'Desc'{data = NData, mime = TMime} = D, [Acc, ND2]) + ?LOG_INFO("Message update ~p", [Msg]), + {NewDesc, NewDesc2} = + lists:foldr( + fun (#'Desc'{data = NData, id = DescId} = D, {Acc, _}) when Lang == [], ID /= [] -> + #'Feature'{value = I} = + case roster:get_data(?USERS_KEY, NData) of + [] -> #'Feature'{key = ?USERS_KEY, value = []}; + FU -> FU + end, + {[case DescId of + ID -> roster:set_data(#'Feature'{key = ?USERS_KEY, value = binary_delpart(I, PhoneId)}, D); + _-> D + end | Acc], []}; + (#'Desc'{data = NData, mime = TMime} = D, [Acc, ND2]) when TMime =:= DMime -> - FUsers = #'Feature'{value = I} = - case roster:get_data(?USERS_KEY, NData) of - [] -> #'Feature'{key = ?USERS_KEY, value = []}; FU -> FU end, - case roster:get_data(?LANG_KEY, NData) of - #'Feature'{value = <<_:8, _/binary>> = TLang} -> - case binary_delpart(I, PhoneId) of - [] when I == [] -> [Acc, []]; - [] -> [Acc, ND2]; - I2 when TLang == Lang -> - [[roster:set_data(FUsers#'Feature'{key = ?USERS_KEY, value = <>}, - D#'Desc'{payload = Payload}) | Acc], []]; - I2 -> - [[roster:set_data(FUsers#'Feature'{key = ?USERS_KEY, value = I2}, - D#'Desc'{}) | Acc], ND2] - end; - _ -> [[D | Acc], ND2] - end; - (D, [Acc, ND2]) -> [[D | Acc], ND2] - end, [[], [roster:set_data(#'Feature'{key = ?USERS_KEY, value = PhoneId}, ND)]], - [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD<-Descs])), + FUsers = #'Feature'{value = I} = + case roster:get_data(?USERS_KEY, NData) of + [] -> #'Feature'{key = ?USERS_KEY, value = []}; + FU -> FU + end, + case roster:get_data(?LANG_KEY, NData) of + #'Feature'{value = <<_:8, _/binary>> = TLang} -> + case binary_delpart(I, PhoneId) of + [] when I == [] -> {Acc, []}; + [] -> {Acc, ND2}; + I2 when TLang == Lang -> + {[roster:set_data(FUsers#'Feature'{key = ?USERS_KEY, value = <>}, + D#'Desc'{payload = Payload}) | Acc], []}; + I2 -> + {[roster:set_data(FUsers#'Feature'{key = ?USERS_KEY, value = I2}, + D#'Desc'{}) | Acc], ND2} + end; + _ -> {[D | Acc], ND2} + end; + (D, {Acc, ND2}) -> + {[D | Acc], ND2} + end, {[], [roster:set_data(#'Feature'{key = ?USERS_KEY, value = PhoneId}, ND)]}, + [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD<-Descs]), + ?LOG_INFO("Message updated after split ~p ~p", [NewDesc, NewDesc2]), + Descs2 = NewDesc ++ NewDesc2, kvs:put(NMsg = Msg#'Message'{files = Descs2}), case kvs_stream:load_writer(Feed) of #writer{cache = #'Message'{id = Id}} = W -> kvs_stream:save(W#writer{cache = NMsg}); _ -> skip end, - roster:send_feed(C, Feed, NMsg#'Message'{status = update}), <<>>; + ?LOG_INFO("Send to ~p (alive ~p)", [C, is_process_alive(C)]), + roster:send_feed(C, Feed, NMsg#'Message'{status = update}), + <<>>; _ -> #io{code = #error{code = invalid_data}} end; _ -> #io{code = #error{code = message_not_found}} end, -- GitLab From 53ce99925432d8e37dda87a2db7d0b3d55f5a1c1 Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Tue, 2 Jun 2020 13:04:02 +0200 Subject: [PATCH 04/15] Extract updating a message as a function --- apps/roster/src/protocol/roster_message.erl | 154 +++++++++++--------- 1 file changed, 84 insertions(+), 70 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 048b0ef66..9f7014fb7 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -347,7 +347,8 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" Pid = n2o_async:pid(system, roster_message), case kvs:get('Message', Id) of {ok, #'Message'{feed_id = Feed, from = From, to = To, files = [#'Desc'{mime = <<"audio">>, payload = Uri} | _]}} -> - ?LOG_DEBUG("enter ~p transcribe process with ~p, (Lang=~p)", [Type, Uri, Lang]), + PhoneId = roster:phone_id(ClientId, From), + ?LOG_DEBUG("enter ~p transcribe process with ~p, (Lang=~p) from ~p", [Type, Uri, Lang, PhoneId]), ErrMsg = #'Message'{id = Id, feed_id = Feed, from = From, to = To}, case Type of short -> @@ -369,7 +370,7 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" ?LOG_ERROR("invalid url for transcribe: ~p ~p", [Uri, ErrInfo]), #io{code = {error, invalid_data}, data = ErrMsg} end, - Pid ! {Res, ClientId} + Pid ! msg_update(Res, ClientId, PhoneId, Lang) end), {reply, {bert, #io{code = #ok{code = transcribe}, data = Id}}, Req, State}; long -> @@ -390,7 +391,7 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" ?LOG_ERROR("invalid url for transcribe: ~p ~p", [Uri, ErrInfo]), #io{code = {error, invalid_data}, data = ErrMsg} end, - Pid ! {Res, ClientId} + Pid ! msg_update(Res, ClientId, PhoneId, Lang) end), {reply, {bert, #io{code = #ok{code = transcribe}, data = Id}}, Req, State}; _ -> @@ -407,68 +408,18 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" info(#'Message'{status = update, id = Id, from = From, - files = [#'Desc'{id = ID, payload = Payload, data = Data, mime = DMime} = ND | _]}, Req, - #cx{params = ClientId, client_pid = C, state=ack} = State) when is_integer(Id) -> + files = [#'Desc'{data = Data} | _]} = Msg, Req, + #cx{params = ClientId, client_pid = C, state = ack} = State) when is_integer(Id) -> PhoneId = case ClientId of ?SYS_SCHED_CLIENT -> From; <<"emqttd_", _/binary>> -> roster:phone_id(ClientId) end, ?ROSTER_LOG_REQ('Message', update, ClientId, "Id=~p", [Id]), Lang = roster:get_data_val(?LANG_KEY, Data), - IO = case kvs:get('Message', Id) of - {ok, #'Message'{feed_id = Feed, files = Descs} = Msg} -> - case roster:is_visible_msg(Msg, PhoneId) of - 1 -> - ?LOG_INFO("Message update ~p", [Msg]), - {NewDesc, NewDesc2} = - lists:foldr( - fun (#'Desc'{data = NData, id = DescId} = D, {Acc, _}) when Lang == [], ID /= [] -> - #'Feature'{value = I} = - case roster:get_data(?USERS_KEY, NData) of - [] -> #'Feature'{key = ?USERS_KEY, value = []}; - FU -> FU - end, - {[case DescId of - ID -> roster:set_data(#'Feature'{key = ?USERS_KEY, value = binary_delpart(I, PhoneId)}, D); - _-> D - end | Acc], []}; - (#'Desc'{data = NData, mime = TMime} = D, [Acc, ND2]) - when TMime =:= DMime -> - FUsers = #'Feature'{value = I} = - case roster:get_data(?USERS_KEY, NData) of - [] -> #'Feature'{key = ?USERS_KEY, value = []}; - FU -> FU - end, - case roster:get_data(?LANG_KEY, NData) of - #'Feature'{value = <<_:8, _/binary>> = TLang} -> - case binary_delpart(I, PhoneId) of - [] when I == [] -> {Acc, []}; - [] -> {Acc, ND2}; - I2 when TLang == Lang -> - {[roster:set_data(FUsers#'Feature'{key = ?USERS_KEY, value = <>}, - D#'Desc'{payload = Payload}) | Acc], []}; - I2 -> - {[roster:set_data(FUsers#'Feature'{key = ?USERS_KEY, value = I2}, - D#'Desc'{}) | Acc], ND2} - end; - _ -> {[D | Acc], ND2} - end; - (D, {Acc, ND2}) -> - {[D | Acc], ND2} - end, {[], [roster:set_data(#'Feature'{key = ?USERS_KEY, value = PhoneId}, ND)]}, - [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD<-Descs]), - ?LOG_INFO("Message updated after split ~p ~p", [NewDesc, NewDesc2]), - Descs2 = NewDesc ++ NewDesc2, - kvs:put(NMsg = Msg#'Message'{files = Descs2}), - case kvs_stream:load_writer(Feed) of - #writer{cache = #'Message'{id = Id}} = W -> kvs_stream:save(W#writer{cache = NMsg}); - _ -> skip - end, - ?LOG_INFO("Send to ~p (alive ~p)", [C, is_process_alive(C)]), - roster:send_feed(C, Feed, NMsg#'Message'{status = update}), - <<>>; - _ -> #io{code = #error{code = invalid_data}} - end; - _ -> #io{code = #error{code = message_not_found}} end, - {reply, {bert, IO}, Req, State}; - + case msg_update(Msg, ClientId, PhoneId, Lang) of + {updated, #'Message'{feed_id = Feed} = NMsg, _} -> + roster:send_feed(C, Feed, NMsg), + {reply, {bert, <<>>}, Req, State}; + {update, #io{} = IO, _} -> + {reply, {bert, IO}, Req, State} + end; info(#'Message'{from = From, to = To, status = Status}, Req, #cx{params = ClientId} = State) -> ?ROSTER_LOG_REQ('Message', Status, ClientId, "Unknown request, From=~p, To=~p", [From, To]), @@ -495,14 +446,14 @@ proc({#io{} = IO, ClientId}, #handler{state = C} = H) -> ?LOG_ERROR("~p", [IO]), roster:send_action(C, ClientId, IO), {reply, [], H}; -proc({#'Message'{status = update} = Msg, ClientId}, #handler{state = C} = H) -> - ?LOG_INFO("UPDATE TRANSCRIBE: ~p", [Msg]), - %% As a dirty hack, we let sys_bpe post the messaage update. The client itself may have disconnected - try {reply, {bert, IO}, _, _} = info(Msg, {[], handled}, #cx{params = ClientId, client_pid = C, state = ack}), - roster:send_action(C, ClientId, IO) - catch Err:Rea:Sta -> - ?LOG_ERROR("~p:~p:~p", [Err, Rea, Sta]) - end, +proc({updated, #'Message'{feed_id = Feed} = Msg, ClientId}, #handler{state = C} = H) -> + ?LOG_INFO("Updated ~p", [Msg]), + roster:send_feed(C, Feed, Msg#'Message'{status = update}), + roster:send_action(C, ClientId, <<>>), + {reply, [], H}; +proc({updated, #io{} = IO, ClientId}, #handler{state = C} = H) -> + ?LOG_ERROR("~p", [IO]), + roster:send_action(C, ClientId, IO), {reply, [], H}; proc({mqttc, C, connected}, State = #handler{state = C, seq = S}) -> {ok, State#handler{seq = S + 1}}; proc({mqttc, _C, disconnected}, State) -> {ok, State}; @@ -574,6 +525,69 @@ msg_preview(#'Message'{files = Attachments}) -> iolist_to_binary([string:to_upper(FirstLetter), LastText]) end. +msg_update(#io{} = IO, ClientId, _PhoneId, _Lang) -> + {IO, ClientId}; +msg_update(#'Message'{id = Id, files = [#'Desc'{id = ID, payload = Payload, mime = DMime} = UpdateDesc | _]}, ClientId, PhoneId, Lang) -> + ?ROSTER_LOG_REQ('Message', update, ClientId, "Id=~p Lang=~p", [Id, Lang]), + %% find most recent version of message... + %% another update could have been ongoing in parallel + case kvs:get('Message', Id) of + {ok, #'Message'{feed_id = Feed, files = Descs} = Msg} -> + case roster:is_visible_msg(Msg, PhoneId) of + 1 -> + %% The 'Feature' with ?USERS_KEY tells the client whether or not to display the update + {NewDesc, NewDesc2} = + lists:foldr( + fun (#'Desc'{data = NData, id = DescId} = D, {Acc, _}) when Lang == [], ID /= [] -> + #'Feature'{value = I} = + case roster:get_data(?USERS_KEY, NData) of + [] -> #'Feature'{key = ?USERS_KEY, value = []}; + FU -> FU + end, + {[case DescId of + ID -> roster:set_data(#'Feature'{key = ?USERS_KEY, value = binary_delpart(I, PhoneId)}, D); + _-> D + end | Acc], []}; + (#'Desc'{data = NData, mime = TMime} = D, [Acc, ND2]) + when TMime =:= DMime -> + FUsers = #'Feature'{value = I} = + case roster:get_data(?USERS_KEY, NData) of + [] -> #'Feature'{key = ?USERS_KEY, value = []}; + FU -> FU + end, + case roster:get_data(?LANG_KEY, NData) of + #'Feature'{value = <<_:8, _/binary>> = TLang} -> + case binary_delpart(I, PhoneId) of + [] when I == [] -> {Acc, []}; + [] -> {Acc, ND2}; + I2 when TLang == Lang -> + {[roster:set_data(FUsers#'Feature'{key = ?USERS_KEY, value = <>}, + D#'Desc'{payload = Payload}) | Acc], []}; + I2 -> + {[roster:set_data(FUsers#'Feature'{key = ?USERS_KEY, value = I2}, + D#'Desc'{}) | Acc], ND2} + end; + _ -> {[D | Acc], ND2} + end; + (D, {Acc, ND2}) -> + {[D | Acc], ND2} + end, {[], [roster:set_data(#'Feature'{key = ?USERS_KEY, value = PhoneId}, UpdateDesc)]}, + [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD <- Descs]), + Descs2 = NewDesc ++ NewDesc2, + NMsg = Msg#'Message'{files = Descs2}, + kvs:put(NMsg), + case kvs_stream:load_writer(Feed) of + #writer{cache = #'Message'{id = Id}} = W -> kvs_stream:save(W#writer{cache = NMsg}); + _ -> skip + end, + {updated, NMsg, ClientId}; + _ -> + {updated, #io{code = #error{code = invalid_data}}, ClientId} + end; + _ -> + {updated, #io{code = #error{code = message_not_found}}, ClientId} + end. + binary_delpart([], _Part) -> []; binary_delpart(Bin, Part) -> List = binary:split(Bin, [<<",">>], [global]), -- GitLab From 8412abc7beaa0a280d7442fd96d7f84e4e95c4a0 Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Thu, 4 Jun 2020 08:06:40 +0200 Subject: [PATCH 05/15] Flatten is needed and update is of the original message --- apps/roster/src/protocol/roster_message.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 9f7014fb7..6453b8fa6 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -347,7 +347,7 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" Pid = n2o_async:pid(system, roster_message), case kvs:get('Message', Id) of {ok, #'Message'{feed_id = Feed, from = From, to = To, files = [#'Desc'{mime = <<"audio">>, payload = Uri} | _]}} -> - PhoneId = roster:phone_id(ClientId, From), + PhoneId = case ClientId of <<"sys_bpe">> -> From; <<"emqttd_", _/binary>> -> roster:phone_id(ClientId) end, ?LOG_DEBUG("enter ~p transcribe process with ~p, (Lang=~p) from ~p", [Type, Uri, Lang, PhoneId]), ErrMsg = #'Message'{id = Id, feed_id = Feed, from = From, to = To}, case Type of @@ -448,7 +448,7 @@ proc({#io{} = IO, ClientId}, #handler{state = C} = H) -> {reply, [], H}; proc({updated, #'Message'{feed_id = Feed} = Msg, ClientId}, #handler{state = C} = H) -> ?LOG_INFO("Updated ~p", [Msg]), - roster:send_feed(C, Feed, Msg#'Message'{status = update}), + roster:send_feed(C, Feed, Msg), roster:send_action(C, ClientId, <<>>), {reply, [], H}; proc({updated, #io{} = IO, ClientId}, #handler{state = C} = H) -> @@ -573,14 +573,14 @@ msg_update(#'Message'{id = Id, files = [#'Desc'{id = ID, payload = Payload, mime {[D | Acc], ND2} end, {[], [roster:set_data(#'Feature'{key = ?USERS_KEY, value = PhoneId}, UpdateDesc)]}, [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD <- Descs]), - Descs2 = NewDesc ++ NewDesc2, + Descs2 = lists:flatten([NewDesc, NewDesc2]), NMsg = Msg#'Message'{files = Descs2}, kvs:put(NMsg), case kvs_stream:load_writer(Feed) of #writer{cache = #'Message'{id = Id}} = W -> kvs_stream:save(W#writer{cache = NMsg}); _ -> skip end, - {updated, NMsg, ClientId}; + {updated, NMsg#'Message'{status = update}, ClientId}; _ -> {updated, #io{code = #error{code = invalid_data}}, ClientId} end; -- GitLab From f75f743e1443cd6394af45467dfd86afc28109ca Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Thu, 4 Jun 2020 09:43:41 +0200 Subject: [PATCH 06/15] Try to move possible race to different context --- apps/roster/src/protocol/roster_message.erl | 40 +++++++++++---------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 6453b8fa6..47fc3078a 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -370,7 +370,7 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" ?LOG_ERROR("invalid url for transcribe: ~p ~p", [Uri, ErrInfo]), #io{code = {error, invalid_data}, data = ErrMsg} end, - Pid ! msg_update(Res, ClientId, PhoneId, Lang) + Pid ! {transcribed, Res, ClientId, PhoneId, Lang} end), {reply, {bert, #io{code = #ok{code = transcribe}, data = Id}}, Req, State}; long -> @@ -391,7 +391,7 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" ?LOG_ERROR("invalid url for transcribe: ~p ~p", [Uri, ErrInfo]), #io{code = {error, invalid_data}, data = ErrMsg} end, - Pid ! msg_update(Res, ClientId, PhoneId, Lang) + Pid ! {transcribed, Res, ClientId, PhoneId, Lang} end), {reply, {bert, #io{code = #ok{code = transcribe}, data = Id}}, Req, State}; _ -> @@ -415,7 +415,7 @@ info(#'Message'{status = update, id = Id, from = From, Lang = roster:get_data_val(?LANG_KEY, Data), case msg_update(Msg, ClientId, PhoneId, Lang) of {updated, #'Message'{feed_id = Feed} = NMsg, _} -> - roster:send_feed(C, Feed, NMsg), + roster:send_feed(C, Feed, NMsg#'Message'{status = update}), {reply, {bert, <<>>}, Req, State}; {update, #io{} = IO, _} -> {reply, {bert, IO}, Req, State} @@ -446,13 +446,17 @@ proc({#io{} = IO, ClientId}, #handler{state = C} = H) -> ?LOG_ERROR("~p", [IO]), roster:send_action(C, ClientId, IO), {reply, [], H}; -proc({updated, #'Message'{feed_id = Feed} = Msg, ClientId}, #handler{state = C} = H) -> - ?LOG_INFO("Updated ~p", [Msg]), - roster:send_feed(C, Feed, Msg), - roster:send_action(C, ClientId, <<>>), +proc({transcribed, #'Message'{} = Msg, ClientId, PhoneId, Lang}, #handler{state = C} = H) -> + case msg_update(Msg, ClientId, PhoneId, Lang) of + {updated, #'Message'{feed_id = Feed} = NMsg, _} -> + ?LOG_INFO("Updated ~p", [NMsg]), + roster:send_feed(C, Feed, NMsg#'Message'{status = update}); + {update, #io{} = IO, _} -> + ?LOG_ERROR("transcribe ~p update error ~p", [Msg, IO]), + roster:send_action(C, ClientId, IO) + end, {reply, [], H}; -proc({updated, #io{} = IO, ClientId}, #handler{state = C} = H) -> - ?LOG_ERROR("~p", [IO]), +proc({transcribed, #io{} = IO, ClientId, _PhoneId, _Lang}, #handler{state = C} = H) -> roster:send_action(C, ClientId, IO), {reply, [], H}; proc({mqttc, C, connected}, State = #handler{state = C, seq = S}) -> {ok, State#handler{seq = S + 1}}; @@ -525,30 +529,28 @@ msg_preview(#'Message'{files = Attachments}) -> iolist_to_binary([string:to_upper(FirstLetter), LastText]) end. -msg_update(#io{} = IO, ClientId, _PhoneId, _Lang) -> - {IO, ClientId}; -msg_update(#'Message'{id = Id, files = [#'Desc'{id = ID, payload = Payload, mime = DMime} = UpdateDesc | _]}, ClientId, PhoneId, Lang) -> +msg_update(#'Message'{id = Id, files = [#'Desc'{id = UpdateDescId, payload = Payload, mime = DMime} = UpdateDesc | _]}, ClientId, PhoneId, Lang) -> ?ROSTER_LOG_REQ('Message', update, ClientId, "Id=~p Lang=~p", [Id, Lang]), %% find most recent version of message... %% another update could have been ongoing in parallel case kvs:get('Message', Id) of - {ok, #'Message'{feed_id = Feed, files = Descs} = Msg} -> + {ok, #'Message'{files = Descs, feed_id = Feed} = Msg} -> case roster:is_visible_msg(Msg, PhoneId) of 1 -> %% The 'Feature' with ?USERS_KEY tells the client whether or not to display the update {NewDesc, NewDesc2} = lists:foldr( - fun (#'Desc'{data = NData, id = DescId} = D, {Acc, _}) when Lang == [], ID /= [] -> + fun (#'Desc'{data = NData, id = DescId} = D, {Acc, _}) when Lang == [], UpdateDescId /= [] -> #'Feature'{value = I} = case roster:get_data(?USERS_KEY, NData) of [] -> #'Feature'{key = ?USERS_KEY, value = []}; FU -> FU end, - {[case DescId of - ID -> roster:set_data(#'Feature'{key = ?USERS_KEY, value = binary_delpart(I, PhoneId)}, D); - _-> D + {[case DescId == UpdateDescId of + true -> roster:set_data(#'Feature'{key = ?USERS_KEY, value = binary_delpart(I, PhoneId)}, D); + false-> D end | Acc], []}; - (#'Desc'{data = NData, mime = TMime} = D, [Acc, ND2]) + (#'Desc'{data = NData, mime = TMime} = D, {Acc, ND2}) when TMime =:= DMime -> FUsers = #'Feature'{value = I} = case roster:get_data(?USERS_KEY, NData) of @@ -580,7 +582,7 @@ msg_update(#'Message'{id = Id, files = [#'Desc'{id = ID, payload = Payload, mime #writer{cache = #'Message'{id = Id}} = W -> kvs_stream:save(W#writer{cache = NMsg}); _ -> skip end, - {updated, NMsg#'Message'{status = update}, ClientId}; + {updated, NMsg, ClientId}; _ -> {updated, #io{code = #error{code = invalid_data}}, ClientId} end; -- GitLab From ed1306902c5bbc5ee741e3a8d9929df22d2642ed Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Fri, 5 Jun 2020 15:34:12 +0200 Subject: [PATCH 07/15] Clarify different cases --- apps/roster/src/protocol/roster_message.erl | 73 +++++++++++++-------- 1 file changed, 45 insertions(+), 28 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 47fc3078a..197c1162c 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -540,42 +540,59 @@ msg_update(#'Message'{id = Id, files = [#'Desc'{id = UpdateDescId, payload = Pay %% The 'Feature' with ?USERS_KEY tells the client whether or not to display the update {NewDesc, NewDesc2} = lists:foldr( - fun (#'Desc'{data = NData, id = DescId} = D, {Acc, _}) when Lang == [], UpdateDescId /= [] -> - #'Feature'{value = I} = - case roster:get_data(?USERS_KEY, NData) of - [] -> #'Feature'{key = ?USERS_KEY, value = []}; - FU -> FU - end, - {[case DescId == UpdateDescId of - true -> roster:set_data(#'Feature'{key = ?USERS_KEY, value = binary_delpart(I, PhoneId)}, D); - false-> D - end | Acc], []}; - (#'Desc'{data = NData, mime = TMime} = D, {Acc, ND2}) + fun (#'Desc'{data = DescData, id = DescId} = D, {Acc, _}) when Lang == [], UpdateDescId /= [] -> + UsersFeature = roster:get_data(?USERS_KEY, DescData), + {case {DescId == UpdateDescId, UsersFeature} of + {true, []} -> + Acc; + {true, #'Feature'{value = PhoneIds}} -> + case binary_delpart(PhoneIds, PhoneId) of + [] -> Acc; + RemainingUsers -> + ?LOG_INFO("Other users using this Desc ~p (~p)", [Msg, D]), + [roster:set_data(#'Feature'{key = ?USERS_KEY, + value = RemainingUsers}, D) | Acc] + end; + {false, _} -> + [D | Acc] + end, []}; + (#'Desc'{data = DescData, mime = TMime} = D, {Acc, ND2}) when TMime =:= DMime -> - FUsers = #'Feature'{value = I} = - case roster:get_data(?USERS_KEY, NData) of - [] -> #'Feature'{key = ?USERS_KEY, value = []}; - FU -> FU - end, - case roster:get_data(?LANG_KEY, NData) of - #'Feature'{value = <<_:8, _/binary>> = TLang} -> - case binary_delpart(I, PhoneId) of - [] when I == [] -> {Acc, []}; - [] -> {Acc, ND2}; - I2 when TLang == Lang -> - {[roster:set_data(FUsers#'Feature'{key = ?USERS_KEY, value = <>}, + UsersFeature = roster:get_data(?USERS_KEY, DescData), + LanguageFeature = roster:get_data(?LANG_KEY, DescData), + case {LanguageFeature, UsersFeature} of + {#'Feature'{}, []} -> + ?LOG_ERROR("Message contains Desc with LANGUAGE but no USERS viewing it ~p (~p)", [Msg, D]), + %% This indicates database polution or unknown feature + {Acc, []}; + {#'Feature'{value = <<_:8, _/binary>> = TLang}, #'Feature'{value = PhoneIds}} -> + case binary_delpart(PhoneIds, PhoneId) of + [] -> + ?LOG_DEBUG("No other users for this Desc, remove it ~p (~p)", [Msg, D]), + {Acc, ND2}; + RemainingUsers when TLang == Lang -> + %% We keep the same language (re-done a translate/transcribe), update result for all + %% We could also just keep it, without change + ?LOG_DEBUG("Add user to other users for this Desc ~p (~p)", [Msg, D]), + {[roster:set_data(UsersFeature#'Feature'{key = ?USERS_KEY, + value = <>}, D#'Desc'{payload = Payload}) | Acc], []}; - I2 -> - {[roster:set_data(FUsers#'Feature'{key = ?USERS_KEY, value = I2}, - D#'Desc'{}) | Acc], ND2} + RemainingUsers -> + ?LOG_DEBUG("Remove user from this Desc ~p (~p)", [Msg, D]), + %% New language added for this user, remove old translation for this user. + %% but keep the new as ND2 + %% Note that RemainingUsers /= [] and hence Desc /= [] + {[roster:set_data(UsersFeature#'Feature'{key = ?USERS_KEY, + value = RemainingUsers}, D) | Acc], ND2} end; - _ -> {[D | Acc], ND2} + _ -> + {[D | Acc], ND2} end; (D, {Acc, ND2}) -> {[D | Acc], ND2} end, {[], [roster:set_data(#'Feature'{key = ?USERS_KEY, value = PhoneId}, UpdateDesc)]}, [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD <- Descs]), - Descs2 = lists:flatten([NewDesc, NewDesc2]), + Descs2 = NewDesc ++ NewDesc2, NMsg = Msg#'Message'{files = Descs2}, kvs:put(NMsg), case kvs_stream:load_writer(Feed) of -- GitLab From b766c70efbbc3bbc5bd4616bc32ec1a25c79d8c8 Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Fri, 5 Jun 2020 16:52:26 +0200 Subject: [PATCH 08/15] refactor in two different functions --- apps/roster/src/protocol/roster_message.erl | 140 +++++++++++--------- 1 file changed, 80 insertions(+), 60 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 197c1162c..8c29adae9 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -529,71 +529,26 @@ msg_preview(#'Message'{files = Attachments}) -> iolist_to_binary([string:to_upper(FirstLetter), LastText]) end. -msg_update(#'Message'{id = Id, files = [#'Desc'{id = UpdateDescId, payload = Payload, mime = DMime} = UpdateDesc | _]}, ClientId, PhoneId, Lang) -> +msg_update(#'Message'{id = Id, files = [#'Desc'{id = UpdateDescId} = UpdateDesc | _]}, ClientId, PhoneId, Lang) -> ?ROSTER_LOG_REQ('Message', update, ClientId, "Id=~p Lang=~p", [Id, Lang]), %% find most recent version of message... %% another update could have been ongoing in parallel + %% The 'Feature' with ?USERS_KEY tells the client whether or not to display the update case kvs:get('Message', Id) of - {ok, #'Message'{files = Descs, feed_id = Feed} = Msg} -> + {ok, #'Message'{feed_id = Feed} = Msg} -> case roster:is_visible_msg(Msg, PhoneId) of 1 -> - %% The 'Feature' with ?USERS_KEY tells the client whether or not to display the update - {NewDesc, NewDesc2} = - lists:foldr( - fun (#'Desc'{data = DescData, id = DescId} = D, {Acc, _}) when Lang == [], UpdateDescId /= [] -> - UsersFeature = roster:get_data(?USERS_KEY, DescData), - {case {DescId == UpdateDescId, UsersFeature} of - {true, []} -> - Acc; - {true, #'Feature'{value = PhoneIds}} -> - case binary_delpart(PhoneIds, PhoneId) of - [] -> Acc; - RemainingUsers -> - ?LOG_INFO("Other users using this Desc ~p (~p)", [Msg, D]), - [roster:set_data(#'Feature'{key = ?USERS_KEY, - value = RemainingUsers}, D) | Acc] - end; - {false, _} -> - [D | Acc] - end, []}; - (#'Desc'{data = DescData, mime = TMime} = D, {Acc, ND2}) - when TMime =:= DMime -> - UsersFeature = roster:get_data(?USERS_KEY, DescData), - LanguageFeature = roster:get_data(?LANG_KEY, DescData), - case {LanguageFeature, UsersFeature} of - {#'Feature'{}, []} -> - ?LOG_ERROR("Message contains Desc with LANGUAGE but no USERS viewing it ~p (~p)", [Msg, D]), - %% This indicates database polution or unknown feature - {Acc, []}; - {#'Feature'{value = <<_:8, _/binary>> = TLang}, #'Feature'{value = PhoneIds}} -> - case binary_delpart(PhoneIds, PhoneId) of - [] -> - ?LOG_DEBUG("No other users for this Desc, remove it ~p (~p)", [Msg, D]), - {Acc, ND2}; - RemainingUsers when TLang == Lang -> - %% We keep the same language (re-done a translate/transcribe), update result for all - %% We could also just keep it, without change - ?LOG_DEBUG("Add user to other users for this Desc ~p (~p)", [Msg, D]), - {[roster:set_data(UsersFeature#'Feature'{key = ?USERS_KEY, - value = <>}, - D#'Desc'{payload = Payload}) | Acc], []}; - RemainingUsers -> - ?LOG_DEBUG("Remove user from this Desc ~p (~p)", [Msg, D]), - %% New language added for this user, remove old translation for this user. - %% but keep the new as ND2 - %% Note that RemainingUsers /= [] and hence Desc /= [] - {[roster:set_data(UsersFeature#'Feature'{key = ?USERS_KEY, - value = RemainingUsers}, D) | Acc], ND2} - end; - _ -> - {[D | Acc], ND2} - end; - (D, {Acc, ND2}) -> - {[D | Acc], ND2} - end, {[], [roster:set_data(#'Feature'{key = ?USERS_KEY, value = PhoneId}, UpdateDesc)]}, - [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD <- Descs]), - Descs2 = NewDesc ++ NewDesc2, - NMsg = Msg#'Message'{files = Descs2}, + NewDescs = + case Lang == [] andalso UpdateDescId /= [] of + true -> + msg_removing_update(Msg, UpdateDescId, PhoneId); + false -> + msg_changing_update(Msg, + roster:set_data(#'Feature'{key = ?USERS_KEY, + value = PhoneId}, UpdateDesc), + Lang, PhoneId) + end, + NMsg = Msg#'Message'{files = NewDescs}, kvs:put(NMsg), case kvs_stream:load_writer(Feed) of #writer{cache = #'Message'{id = Id}} = W -> kvs_stream:save(W#writer{cache = NMsg}); @@ -607,7 +562,72 @@ msg_update(#'Message'{id = Id, files = [#'Desc'{id = UpdateDescId, payload = Pay {updated, #io{code = #error{code = message_not_found}}, ClientId} end. -binary_delpart([], _Part) -> []; +msg_removing_update(#'Message'{files = Descs} = Msg, UpdateDescId, PhoneId) -> + %% There is no clear reason why we should sort features, but client may rely on it. + lists:foldr( + fun (#'Desc'{data = DescData, id = DescId} = D, Acc) -> + UsersFeature = roster:get_data(?USERS_KEY, DescData), + case {DescId == UpdateDescId, UsersFeature} of + {true, []} -> + Acc; + {true, #'Feature'{value = PhoneIds}} -> + case binary_delpart(PhoneIds, PhoneId) of + [] -> Acc; + RemainingUsers -> + ?LOG_DEBUG("Other users using this Desc ~p (~p)", [Msg, D]), + [roster:set_data(#'Feature'{key = ?USERS_KEY, + value = RemainingUsers}, D) | Acc] + end; + {false, _} -> + [D | Acc] + end + end, [], + [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD <- Descs]). + +msg_changing_update(#'Message'{files = Descs} = Msg, #'Desc'{payload = Payload, mime = DMime} = UpdateDesc, Lang, PhoneId) -> + {NewDescs, NewDesc2} = + lists:foldr( + fun (#'Desc'{data = DescData, mime = TMime} = D, {Acc, ND2}) + when TMime =:= DMime -> + UsersFeature = roster:get_data(?USERS_KEY, DescData), + LanguageFeature = roster:get_data(?LANG_KEY, DescData), + case {LanguageFeature, UsersFeature} of + {#'Feature'{}, []} -> + ?LOG_ERROR("Message contains Desc with LANGUAGE but no USERS viewing it ~p (~p)", [Msg, D]), + %% This indicates database polution or unknown feature + {Acc, []}; + {#'Feature'{value = <<_:8, _/binary>> = TLang}, #'Feature'{value = PhoneIds}} -> + case binary_delpart(PhoneIds, PhoneId) of + [] -> + ?LOG_DEBUG("No other users for this Desc, remove it ~p (~p)", [Msg, D]), + {Acc, ND2}; + RemainingUsers when TLang == Lang -> + %% We keep the same language (re-done a translate/transcribe), update result for all + %% We could also just keep it, without change of payload, but client may have better + %% translation available? + ?LOG_DEBUG("Add user to other users for this Desc ~p (~p)", [Msg, D]), + {[roster:set_data(UsersFeature#'Feature'{key = ?USERS_KEY, + value = <>}, + D#'Desc'{payload = Payload}) | Acc], []}; + RemainingUsers -> + ?LOG_DEBUG("Remove user from this Desc ~p (~p)", [Msg, D]), + %% New language added for this user, remove old translation for this user. + %% but keep the new as ND2 + %% Note that RemainingUsers /= [] and hence Desc /= [] + {[roster:set_data(UsersFeature#'Feature'{key = ?USERS_KEY, + value = RemainingUsers}, D) | Acc], ND2} + end; + _ -> + {[D | Acc], ND2} + end; + (D, {Acc, ND2}) -> + {[D | Acc], ND2} + end, {[], [UpdateDesc]}, + [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD <- Descs]), + NewDescs ++ NewDesc2. + +binary_delpart([], _Part) -> + []; binary_delpart(Bin, Part) -> List = binary:split(Bin, [<<",">>], [global]), case lists:reverse(List--[Part]) of -- GitLab From a689cb17aa426f2ab0b29bcf9298fc7751e10cd9 Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Fri, 5 Jun 2020 14:26:18 +0200 Subject: [PATCH 09/15] no `binary_to_atom` just to be sure --- apps/roster/src/protocol/roster_message.erl | 30 +++++++++++++-------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 8c29adae9..4c5e6f1db 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -338,26 +338,34 @@ info(#'Message'{status = update, type = [draft], feed_id = Feed0, from = From, t end, {reply, {bert, IO}, Req, State}; -info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe">>, payload = [], data = Data} = Desc | _]} = Msg, Req, - #cx{params = ClientId, state = ack} = State) - when is_integer(Id), not is_tuple(Req) -> +info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe">>, payload = [], data = Data} = Desc | _]} = Msg, + Req, + #cx{params = ClientId, state = ack} = State) when is_integer(Id), not is_tuple(Req) -> ?ROSTER_LOG_REQ('Message', update, ClientId, "Type=transcribe, Id=~p", [Id]), - Type = case roster:get_data_val(<<"TYPE">>, Data) of [] -> short; T -> binary_to_atom(T, utf8) end, + Type = + case roster:get_data_val(<<"TYPE">>, Data) of + [] -> <<"short">>; + T -> string:lowercase(T) + end, Lang = roster:get_data_val(?LANG_KEY, Data), Pid = n2o_async:pid(system, roster_message), case kvs:get('Message', Id) of {ok, #'Message'{feed_id = Feed, from = From, to = To, files = [#'Desc'{mime = <<"audio">>, payload = Uri} | _]}} -> - PhoneId = case ClientId of <<"sys_bpe">> -> From; <<"emqttd_", _/binary>> -> roster:phone_id(ClientId) end, - ?LOG_DEBUG("enter ~p transcribe process with ~p, (Lang=~p) from ~p", [Type, Uri, Lang, PhoneId]), + PhoneId = + case ClientId of + <<"sys_bpe">> -> From; + <<"emqttd_", _/binary>> -> roster:phone_id(ClientId) + end, + ?LOG_DEBUG("enter ~s transcribe process with ~p, (Lang=~p) from ~p", [Type, Uri, Lang, PhoneId]), ErrMsg = #'Message'{id = Id, feed_id = Feed, from = From, to = To}, case Type of - short -> + <<"short">> -> spawn(fun() -> Res = case google_api:convert_ffmpeg(binary_to_list(Uri)) of {file, FileOut, FileIn} -> {ok, Binary} = file:read_file(FileOut), - R = case google_api:transcribe(Type, base64:encode(Binary), Lang) of + R = case google_api:transcribe(short, base64:encode(Binary), Lang) of {error, _} = E -> #io{code = E, data = ErrMsg}; Text -> @@ -373,12 +381,12 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" Pid ! {transcribed, Res, ClientId, PhoneId, Lang} end), {reply, {bert, #io{code = #ok{code = transcribe}, data = Id}}, Req, State}; - long -> + <<"long">> -> spawn(fun() -> Res = case google_api:gs_upload(binary_to_list(Uri)) of {gs, GsUri} -> - R = case google_api:transcribe(Type, GsUri, Lang) of + R = case google_api:transcribe(long, GsUri, Lang) of {error, _} = E -> #io{code = E, data = ErrMsg}; Text -> @@ -395,7 +403,7 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" end), {reply, {bert, #io{code = #ok{code = transcribe}, data = Id}}, Req, State}; _ -> - ?LOG_ERROR("invalid transcribe type ~p", [Type]), + ?LOG_ERROR("invalid transcribe type ~s", [Type]), {reply, {bert, #io{code = #error{code = invalid_data}, data = ErrMsg}}, Req, State} end; {ok, InvalidMsg} -> -- GitLab From 8f1abd4f98b6b8dbe88e294e456436122b44cb07 Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Fri, 5 Jun 2020 15:53:18 +0200 Subject: [PATCH 10/15] this is what delpart does --- apps/roster/src/protocol/roster_message.erl | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 4c5e6f1db..bf1c40284 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -637,12 +637,9 @@ msg_changing_update(#'Message'{files = Descs} = Msg, #'Desc'{payload = Payload, binary_delpart([], _Part) -> []; binary_delpart(Bin, Part) -> - List = binary:split(Bin, [<<",">>], [global]), - case lists:reverse(List--[Part]) of + case string:split(Bin, ",", all) -- [Part] of [] -> []; - [O] -> <>; - [H | L] -> T = lists:foldl(fun(A, B) -> <> end, <<>>, L), - <> + RemainingParts -> iolist_to_binary(lists:join(",", RemainingParts)) end. has_flag(Descs, message_ack) -> lists:keymember(<<"ack">>, #'Desc'.mime, Descs). -- GitLab From b7668b4fb5c290dbc37be747e4b629165838b70a Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Fri, 5 Jun 2020 14:21:16 +0200 Subject: [PATCH 11/15] Release notes --- doc/release-notes/next/NY8653-transcribe | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/release-notes/next/NY8653-transcribe b/doc/release-notes/next/NY8653-transcribe index b8251c727..ffed3b5ac 100644 --- a/doc/release-notes/next/NY8653-transcribe +++ b/doc/release-notes/next/NY8653-transcribe @@ -2,10 +2,13 @@ ### Highlights -* Fixed bugs in transcribing long messages -* -- +* Fixed bugs in transcribing long messages NY-8653 ### List of changes * Added tests for transcribe +* Added tests for translate * fixed enenra package to not fail on token refresh +* fixed bug such that closing tab while transcribe/translate in progress results in transcription +* partly addresses NY-10444: longer messages may hit google api time limit for translation and google does not always translate/transcribes right (or actually almost never). + -- GitLab From 7e0d8d558bf6172b65e9178b4f4d220ec37ca6c0 Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Sun, 7 Jun 2020 08:49:44 +0200 Subject: [PATCH 12/15] Sort only once --- apps/roster/src/protocol/roster_message.erl | 39 +++++++++++---------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index bf1c40284..bb1496747 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -543,26 +543,30 @@ msg_update(#'Message'{id = Id, files = [#'Desc'{id = UpdateDescId} = UpdateDesc %% another update could have been ongoing in parallel %% The 'Feature' with ?USERS_KEY tells the client whether or not to display the update case kvs:get('Message', Id) of - {ok, #'Message'{feed_id = Feed} = Msg} -> + {ok, #'Message'{feed_id = Feed, files = Descs} = Msg} -> case roster:is_visible_msg(Msg, PhoneId) of 1 -> + %% There is no clear reason why we should sort features, but client may rely on it. + DescsFS = + [ Desc#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || + #'Desc'{data = DData} = Desc <- Descs ], NewDescs = case Lang == [] andalso UpdateDescId /= [] of true -> - msg_removing_update(Msg, UpdateDescId, PhoneId); + descs_removing_update(DescsFS, UpdateDescId, PhoneId); false -> - msg_changing_update(Msg, + descs_changing_update(DescsFS, roster:set_data(#'Feature'{key = ?USERS_KEY, value = PhoneId}, UpdateDesc), Lang, PhoneId) end, - NMsg = Msg#'Message'{files = NewDescs}, - kvs:put(NMsg), + NewMsg = Msg#'Message'{files = NewDescs}, + kvs:put(NewMsg), case kvs_stream:load_writer(Feed) of - #writer{cache = #'Message'{id = Id}} = W -> kvs_stream:save(W#writer{cache = NMsg}); + #writer{cache = #'Message'{id = Id}} = W -> kvs_stream:save(W#writer{cache = NewMsg}); _ -> skip end, - {updated, NMsg, ClientId}; + {updated, NewMsg, ClientId}; _ -> {updated, #io{code = #error{code = invalid_data}}, ClientId} end; @@ -570,8 +574,7 @@ msg_update(#'Message'{id = Id, files = [#'Desc'{id = UpdateDescId} = UpdateDesc {updated, #io{code = #error{code = message_not_found}}, ClientId} end. -msg_removing_update(#'Message'{files = Descs} = Msg, UpdateDescId, PhoneId) -> - %% There is no clear reason why we should sort features, but client may rely on it. +descs_removing_update(Descs, UpdateDescId, PhoneId) -> lists:foldr( fun (#'Desc'{data = DescData, id = DescId} = D, Acc) -> UsersFeature = roster:get_data(?USERS_KEY, DescData), @@ -582,17 +585,16 @@ msg_removing_update(#'Message'{files = Descs} = Msg, UpdateDescId, PhoneId) -> case binary_delpart(PhoneIds, PhoneId) of [] -> Acc; RemainingUsers -> - ?LOG_DEBUG("Other users using this Desc ~p (~p)", [Msg, D]), + ?LOG_DEBUG("Other users using this Desc ~p (~p)", [D, Descs]), [roster:set_data(#'Feature'{key = ?USERS_KEY, value = RemainingUsers}, D) | Acc] end; {false, _} -> [D | Acc] end - end, [], - [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD <- Descs]). + end, [], Descs). -msg_changing_update(#'Message'{files = Descs} = Msg, #'Desc'{payload = Payload, mime = DMime} = UpdateDesc, Lang, PhoneId) -> +descs_changing_update(Descs, #'Desc'{payload = Payload, mime = DMime} = UpdateDesc, Lang, PhoneId) -> {NewDescs, NewDesc2} = lists:foldr( fun (#'Desc'{data = DescData, mime = TMime} = D, {Acc, ND2}) @@ -601,24 +603,24 @@ msg_changing_update(#'Message'{files = Descs} = Msg, #'Desc'{payload = Payload, LanguageFeature = roster:get_data(?LANG_KEY, DescData), case {LanguageFeature, UsersFeature} of {#'Feature'{}, []} -> - ?LOG_ERROR("Message contains Desc with LANGUAGE but no USERS viewing it ~p (~p)", [Msg, D]), + ?LOG_ERROR("Message contains Desc with LANGUAGE but no USERS viewing it ~p (~p)", [D, Descs]), %% This indicates database polution or unknown feature {Acc, []}; {#'Feature'{value = <<_:8, _/binary>> = TLang}, #'Feature'{value = PhoneIds}} -> case binary_delpart(PhoneIds, PhoneId) of [] -> - ?LOG_DEBUG("No other users for this Desc, remove it ~p (~p)", [Msg, D]), + ?LOG_DEBUG("No other users for this Desc, remove it ~p (~p)", [D, Descs]), {Acc, ND2}; RemainingUsers when TLang == Lang -> %% We keep the same language (re-done a translate/transcribe), update result for all %% We could also just keep it, without change of payload, but client may have better %% translation available? - ?LOG_DEBUG("Add user to other users for this Desc ~p (~p)", [Msg, D]), + ?LOG_DEBUG("Add user to other users for this Desc ~p (~p)", [D, Descs]), {[roster:set_data(UsersFeature#'Feature'{key = ?USERS_KEY, value = <>}, D#'Desc'{payload = Payload}) | Acc], []}; RemainingUsers -> - ?LOG_DEBUG("Remove user from this Desc ~p (~p)", [Msg, D]), + ?LOG_DEBUG("Remove user from this Desc ~p (~p)", [D, Descs]), %% New language added for this user, remove old translation for this user. %% but keep the new as ND2 %% Note that RemainingUsers /= [] and hence Desc /= [] @@ -630,8 +632,7 @@ msg_changing_update(#'Message'{files = Descs} = Msg, #'Desc'{payload = Payload, end; (D, {Acc, ND2}) -> {[D | Acc], ND2} - end, {[], [UpdateDesc]}, - [DD#'Desc'{data = lists:keysort(#'Feature'.key, DData)} || #'Desc'{data = DData} = DD <- Descs]), + end, {[], [UpdateDesc]}, Descs), NewDescs ++ NewDesc2. binary_delpart([], _Part) -> -- GitLab From 42e3695eda6ce4e2d0aca462686ac67481839472 Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Sun, 7 Jun 2020 08:49:54 +0200 Subject: [PATCH 13/15] unrelated refactoring --- apps/roster/src/protocol/roster_message.erl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index bb1496747..8b0abb5da 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -309,7 +309,10 @@ info(#'Message'{id = Id, msg_id = ClMID, feed_id = Feed, from = From0, seenby = info(#'Message'{status = update, type = [draft], feed_id = Feed0, from = From, to = To, files = File}=M, Req, #cx{params = ClientId, client_pid = C, state=ack} = State) -> - PhoneId = case ClientId of ?SYS_SCHED_CLIENT -> From; <<"emqttd_", _/binary>> -> roster:phone_id(ClientId) end, + PhoneId = + case ClientId of ?SYS_SCHED_CLIENT -> From; + <<"emqttd_", _/binary>> -> roster:phone_id(ClientId) + end, ?ROSTER_LOG_REQ('Message', update, ClientId, "Type=draft, Feed=~p, Files=~p", [Feed0, [PL || #'Desc'{payload = PL} <- File]]), {Feed,M1,D}=case File of -- GitLab From f8da548e1b0a172c9a5161bca3be72095f4e4dff Mon Sep 17 00:00:00 2001 From: Thomas Arts Date: Mon, 8 Jun 2020 10:49:23 +0200 Subject: [PATCH 14/15] One more sys_sched instead of sys_bpe --- apps/roster/src/protocol/roster_message.erl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 8b0abb5da..5831b875e 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -310,7 +310,8 @@ info(#'Message'{status = update, type = [draft], feed_id = Feed0, from = From, t files = File}=M, Req, #cx{params = ClientId, client_pid = C, state=ack} = State) -> PhoneId = - case ClientId of ?SYS_SCHED_CLIENT -> From; + case ClientId of + ?SYS_SCHED_CLIENT -> From; <<"emqttd_", _/binary>> -> roster:phone_id(ClientId) end, ?ROSTER_LOG_REQ('Message', update, ClientId, "Type=draft, Feed=~p, Files=~p", @@ -356,7 +357,7 @@ info(#'Message'{status = update, id = Id, files = [#'Desc'{mime = <<"transcribe" {ok, #'Message'{feed_id = Feed, from = From, to = To, files = [#'Desc'{mime = <<"audio">>, payload = Uri} | _]}} -> PhoneId = case ClientId of - <<"sys_bpe">> -> From; + ?SYS_SCHED_CLIENT -> From; <<"emqttd_", _/binary>> -> roster:phone_id(ClientId) end, ?LOG_DEBUG("enter ~s transcribe process with ~p, (Lang=~p) from ~p", [Type, Uri, Lang, PhoneId]), -- GitLab From afa0409057356f75deeb246b9be874221b4ada73 Mon Sep 17 00:00:00 2001 From: Ulf Norell Date: Mon, 8 Jun 2020 11:01:58 +0200 Subject: [PATCH 15/15] Do the right thing in case of db corruption --- apps/roster/src/protocol/roster_message.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 5831b875e..e1350816c 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -609,7 +609,7 @@ descs_changing_update(Descs, #'Desc'{payload = Payload, mime = DMime} = UpdateDe {#'Feature'{}, []} -> ?LOG_ERROR("Message contains Desc with LANGUAGE but no USERS viewing it ~p (~p)", [D, Descs]), %% This indicates database polution or unknown feature - {Acc, []}; + {Acc, ND2}; {#'Feature'{value = <<_:8, _/binary>> = TLang}, #'Feature'{value = PhoneIds}} -> case binary_delpart(PhoneIds, PhoneId) of [] -> -- GitLab