Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Inbox/adhoc fixes #3500

Merged
merged 7 commits into from
Jan 13, 2022
Merged

Inbox/adhoc fixes #3500

merged 7 commits into from
Jan 13, 2022

Conversation

NelsonVides
Copy link
Collaborator

Extract from #3462 code that can be considered neutral or plain improvements over the existing code, so that it can be merged to master promptly.

@NelsonVides
[tests] Fix test that duplicates the message id
8ada5e5 
Inbox kinda depends on IDs not colliding
@NelsonVides NelsonVides force-pushed the inbox/adhoc_fixes branch 2 times, most recently from 509f790 to ee5c39c Compare January 13, 2022 17:34
@codecov
Copy link

codecov bot commented Jan 13, 2022 •
edited
Loading

Codecov Report

Merging #3500 (34f38f6) into master (1f73b71) will decrease coverage by 0.00%.
The diff coverage is 89.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3500      +/-   ##
==========================================
- Coverage   81.00%   80.99%   -0.01%     
==========================================
  Files         418      418              
  Lines       32333    32342       +9     
==========================================
+ Hits        26190    26195       +5     
- Misses       6143     6147       +4
Impacted Files Coverage Δ
src/rdbms/rdbms_queries.erl 88.54% <81.81%> (-2.37%) ⬇️
src/inbox/mod_inbox_rdbms.erl 93.71% <100.00%> (+0.03%) ⬆️
src/inbox/mod_inbox_utils.erl 98.48% <100.00%> (ø)
src/http_upload/mod_http_upload.erl 94.62% <0.00%> (-2.16%) ⬇️
...bal_distrib/mod_global_distrib_hosts_refresher.erl 71.69% <0.00%> (-1.89%) ⬇️
src/rdbms/mongoose_rdbms.erl 62.17% <0.00%> (-1.13%) ⬇️
src/mod_muc_room.erl 77.09% <0.00%> (-0.18%) ⬇️
src/mod_muc_log.erl 78.11% <0.00%> (ø)
src/pubsub/mod_pubsub.erl 73.31% <0.00%> (+0.17%) ⬆️
... and 3 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 1f73b71...34f38f6. Read the comment docs.

@mongoose-im

This comment has been minimized.

Sign in to view
@mongoose-im mongoose-im mentioned this pull request Jan 13, 2022
Closed
arcusfelis
arcusfelis approved these changes Jan 13, 2022
View reviewed changes
Copy link
Contributor

@arcusfelis arcusfelis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, added just questionable comments :D

@mongoose-im

This comment has been minimized.

Sign in to view
@NelsonVides
[tests] Simplify receives in inbox tests
024587b
@NelsonVides
[tests] Enforce Alices' inbox, not Bob's
fb5292b
@NelsonVides
[tests] Make sure the inbox does exist before resetting it
8f01f60
@NelsonVides
[tests] Fix how markers are configured in inbox big_tests
b52709e 
In 3dc8d39, we made markers not be
converted from binaries to atoms and back, but we didn't change that in
big_tests. To make it worse, big_tests were not even configuring the
right key, `reset_markers`, not `markers`.
@NelsonVides
[src] Module not necessary here
96a0dbd
@NelsonVides
[RDBMS] Make increments configurable
34f38f6 
This required changing a bit how queries are prepared, because the mssql
backend is manually matching input fields with the fields as described
in the DB, so it needs to distinguish which fields it needs to retrieve
information from, and which it needs to add to the final query.
@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 13, 2022 •
edited
Loading

small_tests_24 / small_tests / 34f38f6
Reports root / small

small_tests_23 / small_tests / 34f38f6
Reports root / small

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 34f38f6
Reports root/ big
OK: 2690 / Failed: 0 / User-skipped: 231 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 34f38f6
Reports root/ big
OK: 2690 / Failed: 0 / User-skipped: 231 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 34f38f6
Reports root/ big
OK: 2673 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 34f38f6
Reports root/ big
OK: 2690 / Failed: 0 / User-skipped: 231 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 34f38f6
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 34f38f6
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 34f38f6
Reports root/ big
OK: 1541 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 34f38f6
Reports root/ big
OK: 3077 / Failed: 0 / User-skipped: 240 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 34f38f6
Reports root/ big
OK: 3077 / Failed: 0 / User-skipped: 240 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 34f38f6
Reports root/ big
OK: 1834 / Failed: 0 / User-skipped: 361 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 34f38f6
Reports root/ big
OK: 3072 / Failed: 0 / User-skipped: 245 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 34f38f6
Reports root/ big
OK: 3077 / Failed: 0 / User-skipped: 240 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 34f38f6
Reports root/ big
OK: 1680 / Failed: 0 / User-skipped: 362 / Auto-skipped: 0

@arcusfelis arcusfelis merged commit 74c6d93 into master Jan 13, 2022
@arcusfelis arcusfelis deleted the inbox/adhoc_fixes branch January 13, 2022 20:05
@Premwoik Premwoik added this to the 5.1.0 milestone May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arcusfelis arcusfelis Awaiting requested review from arcusfelis

Assignees
No one assigned
Labels
ready
Projects
None yet
Milestone
5.1.0
Development

Successfully merging this pull request may close these issues.
4 participants
@NelsonVides @mongoose-im @arcusfelis @Premwoik