[markers] Implement all configuration for the smart_markers #3546

NelsonVides · 2022-02-16T15:54:59Z

Implement the functionality as described in #3535

codecov · 2022-02-16T16:04:25Z

Codecov Report

Merging #3546 (29480e0) into feature/smart_markers (ef8b27c) will increase coverage by 0.06%.
The diff coverage is 97.77%.

@@                    Coverage Diff                    @@
##           feature/smart_markers    #3546      +/-   ##
=========================================================
+ Coverage                  80.83%   80.89%   +0.06%     
=========================================================
  Files                        425      426       +1     
  Lines                      32273    32351      +78     
=========================================================
+ Hits                       26087    26171      +84     
+ Misses                      6186     6180       -6

Impacted Files	Coverage Δ
src/config/mongoose_config_spec.erl	`99.21% <ø> (ø)`
...rc/smart_markers/mod_smart_markers_rdbms_async.erl	`92.59% <92.59%> (ø)`
src/inbox/mod_inbox.erl	`87.77% <100.00%> (ø)`
src/offline/mod_offline_chatmarkers.erl	`90.76% <100.00%> (ø)`
src/smart_markers/mod_smart_markers.erl	`95.55% <100.00%> (+4.81%)`	⬆️
src/smart_markers/mod_smart_markers_backend.erl	`100.00% <100.00%> (+5.00%)`	⬆️
src/smart_markers/mod_smart_markers_rdbms.erl	`94.54% <100.00%> (+2.44%)`	⬆️
src/cassandra/mongoose_cassandra.erl	`77.77% <0.00%> (-3.71%)`	⬇️
src/mam/mod_mam_muc_rdbms_arch_async.erl	`82.85% <0.00%> (-2.86%)`	⬇️
src/cassandra/mongoose_cassandra_worker.erl	`65.72% <0.00%> (-2.82%)`	⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef8b27c...29480e0. Read the comment docs.

mongoose-im · 2022-03-11T16:09:05Z

small_tests_24 / small_tests / e0086bf
Reports root / small

small_tests_23 / small_tests / e0086bf
Reports root / small

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / e0086bf
Reports root/ big
OK: 2800 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / e0086bf
Reports root/ big
OK: 2800 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / e0086bf
Reports root/ big
OK: 2783 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / e0086bf
Reports root/ big
OK: 2800 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / e0086bf
Reports root/ big
OK: 1504 / Failed: 0 / User-skipped: 399 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / e0086bf
Reports root/ big
OK: 1504 / Failed: 0 / User-skipped: 399 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / e0086bf
Reports root/ big
OK: 3174 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / e0086bf
Reports root/ big
OK: 3174 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / e0086bf
Reports root/ big
OK: 1846 / Failed: 0 / User-skipped: 366 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / e0086bf
Reports root/ big
OK: 1545 / Failed: 0 / User-skipped: 358 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / e0086bf
Reports root/ big
OK: 3169 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / e0086bf
Reports root/ big
OK: 3174 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / e0086bf
Reports root/ big
OK: 1692 / Failed: 0 / User-skipped: 365 / Auto-skipped: 0

arcusfelis

ok, added some comments, otherwise - ok

arcusfelis · 2022-03-14T17:44:36Z

src/smart_markers/mod_smart_markers_rdbms.erl

+                           Private :: boolean()) -> [mod_smart_markers:chat_marker()].
+get_conv_chat_marker(HostType, From, To = #jid{lserver = TLS}, Thread, TS, Private) ->
+    % If To is a room, we'll want to check just the room
+    case mongoose_domain_api:get_subdomain_host_type(TLS) of


ToLServer or ToServer (TLS and TLU are confusing)

arcusfelis · 2022-03-14T17:46:55Z

src/smart_markers/mod_smart_markers_rdbms.erl

+        type => decode_type(Type),
+        timestamp => decode_timestamp(MsgTS),
+        id => MsgId}
+      || {LS, LU, ToJid, MsgThread, Type, MsgId, MsgTS} <- ChatMarkers].


tuple_to_map helper function would be useful here (also code dedup)

arcusfelis · 2022-03-14T17:49:05Z

src/smart_markers/mod_smart_markers_rdbms_async.erl

@@ -0,0 +1,118 @@
+%%%----------------------------------------------------------------------------
+%%% @copyright (C) 2020, Erlang Solutions Ltd.
+%%% @doc


@doc on one line

arcusfelis · 2022-03-14T19:42:36Z

big_tests/tests/smart_markers_SUITE.erl

+        Msgs = [ escalus:wait_for_stanza(User) || User <- Users],
+        get_id(hd(Msgs), MsgId).
+
+mark_msg(Users, RoomBinJid, Marker, StzId) ->


just StanzaId, there is zero benefits to use a short form

arcusfelis · 2022-03-14T19:42:48Z

big_tests/tests/smart_markers_SUITE.erl

        escalus:send(Writer, Msg),
-        [ escalus:wait_for_stanza(User) || User <- Users],
+        Msgs = [ escalus:wait_for_stanza(User) || User <- Users],


arcusfelis · 2022-03-14T19:46:17Z

big_tests/tests/smart_markers_SUITE.erl

+                          _ -> [{<<"after">>, After}]
+                      end,
+        Iq = iq_fetch_marker(MarkedUserBJid ++ MaybeThread ++ MaybeAfter),
+        mongoose_helper:wait_until(


wait until which waits for what exactly? (probably for not a crash, but still)

Until it all returns fine, without crashing. There's a bunch of asserts inside that might fail but are eventually working, it just needs to fetch the iq again.

arcusfelis · 2022-03-14T19:47:18Z

big_tests/tests/smart_markers_SUITE.erl

+                       <<"type">>, <<"groupchat">>),
+        [ begin
+              escalus:send(User, ChatMarker),
+              {ok, MUser} = verify_marker_fetch(User, RoomBinJid),


MUser is a bad name for a list of elements.

arcusfelis · 2022-03-14T19:50:28Z

src/smart_markers/mod_smart_markers.erl

+    Req = maps:from_list(SubEl#xmlel.attrs),
+    MaybePeer = jid:from_binary(maps:get(<<"peer">>, Req, undefined)),
+    MaybeAfter = parse_ts(maps:get(<<"after">>, Req, undefined)),
+    MaybeTread = maps:get(<<"thread">>, Req, undefined),


MaybeThread

arcusfelis · 2022-03-14T19:56:19Z

src/smart_markers/mod_smart_markers_rdbms_async.erl

+
+-spec start_pool(mongooseim:host_type(), mongoose_async_pools:pool_opts()) -> term().
+start_pool(HostType, Opts) ->
+    catch mongoose_async_pools:start_pool(HostType, smart_markers, Opts).


report errors?

This only happens during testing, where pools are turned on and off. For some reason out of the scope of this PR ejabberd_sup was still having the children somewhere there.

mongoose-im · 2022-03-14T21:58:27Z

small_tests_24 / small_tests / 29480e0
Reports root / small

small_tests_23 / small_tests / 29480e0
Reports root / small

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 29480e0
Reports root/ big
OK: 2800 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 29480e0
Reports root/ big
OK: 2800 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 29480e0
Reports root/ big
OK: 2783 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 29480e0
Reports root/ big
OK: 1504 / Failed: 0 / User-skipped: 399 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 29480e0
Reports root/ big
OK: 1545 / Failed: 0 / User-skipped: 358 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 29480e0
Reports root/ big
OK: 1504 / Failed: 0 / User-skipped: 399 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 29480e0
Reports root/ big
OK: 2800 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 29480e0
Reports root/ big
OK: 3174 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 29480e0
Reports root/ big
OK: 1846 / Failed: 0 / User-skipped: 366 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 29480e0
Reports root/ big
OK: 3174 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 29480e0
Reports root/ big
OK: 3169 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 29480e0
Reports root/ big
OK: 3174 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 29480e0
Reports root/ big
OK: 1692 / Failed: 0 / User-skipped: 365 / Auto-skipped: 0

This comment was marked as outdated.

Sign in to view

mongoose-im mentioned this pull request Feb 16, 2022

CI build failures summary #3360

Closed

NelsonVides force-pushed the markers/removals branch from f83207a to 66595bd Compare February 24, 2022 11:17

Base automatically changed from markers/removals to master February 24, 2022 11:43

NelsonVides changed the base branch from master to feature/smart_markers February 24, 2022 19:25

NelsonVides force-pushed the markers/iq branch from 1e68321 to c8c955d Compare February 24, 2022 19:26

This comment was marked as outdated.

Sign in to view

NelsonVides force-pushed the markers/iq branch from c8c955d to 1850750 Compare March 11, 2022 09:07

This comment was marked as outdated.

Sign in to view

Implement config flags and move mod_smart_markers to opts as maps

37c2ef1

NelsonVides force-pushed the markers/iq branch from 1850750 to 6a1ddaa Compare March 11, 2022 11:24

NelsonVides added the waiting-for-review label Mar 11, 2022

NelsonVides changed the title ~~Implement all configuration for the smart_markers~~ [markers] Implement all configuration for the smart_markers Mar 11, 2022

This comment was marked as outdated.

Sign in to view

NelsonVides force-pushed the markers/iq branch from 33b4267 to e0086bf Compare March 11, 2022 16:00

arcusfelis reviewed Mar 14, 2022

View reviewed changes

NelsonVides added 2 commits March 14, 2022 22:42

Implement all configuration for the smart_markers

b6a46e5

Implement async pool for smart_markers

29480e0

NelsonVides force-pushed the markers/iq branch from e0086bf to 29480e0 Compare March 14, 2022 21:50

arcusfelis approved these changes Mar 15, 2022

View reviewed changes

arcusfelis merged commit 02b6471 into feature/smart_markers Mar 15, 2022

arcusfelis deleted the markers/iq branch March 15, 2022 11:03

NelsonVides mentioned this pull request Mar 15, 2022

Feature/smart markers #3590

Merged

NelsonVides removed the waiting-for-review label Mar 16, 2022

Premwoik added this to the 5.1.0 milestone May 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[markers] Implement all configuration for the smart_markers #3546

[markers] Implement all configuration for the smart_markers #3546

NelsonVides commented Feb 16, 2022 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

codecov bot commented Feb 16, 2022 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

mongoose-im commented Mar 11, 2022 •

edited

Loading

arcusfelis left a comment

arcusfelis Mar 14, 2022

arcusfelis Mar 14, 2022

arcusfelis Mar 14, 2022

arcusfelis Mar 14, 2022

arcusfelis Mar 14, 2022

arcusfelis Mar 14, 2022

NelsonVides Mar 14, 2022

arcusfelis Mar 14, 2022

arcusfelis Mar 14, 2022

arcusfelis Mar 14, 2022

NelsonVides Mar 14, 2022

mongoose-im commented Mar 14, 2022 •

edited

Loading

[markers] Implement all configuration for the smart_markers #3546

[markers] Implement all configuration for the smart_markers #3546

Conversation

NelsonVides commented Feb 16, 2022 • edited Loading

This comment was marked as outdated.

This comment was marked as outdated.

codecov bot commented Feb 16, 2022 • edited Loading

Codecov Report

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

mongoose-im commented Mar 11, 2022 • edited Loading

arcusfelis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mongoose-im commented Mar 14, 2022 • edited Loading

NelsonVides commented Feb 16, 2022 •

edited

Loading

codecov bot commented Feb 16, 2022 •

edited

Loading

mongoose-im commented Mar 11, 2022 •

edited

Loading

mongoose-im commented Mar 14, 2022 •

edited

Loading