[#57, #58] feat(presence): implement presence functionality #901

Open
M0dEx wants to merge 4 commits from presence into next
M0dEx commented 2023-05-28 22:10:33 +00:00 (Migrated from gitlab.com)

This PR adds presence functionality (by default disabled), including configurable intervals and idle/offline timeouts.

The only caveat here is that presence events only get batched together when PDUs are sent, somewhat reducing the reliability of presence over federation. This will however require addition of a generalized functionality for reliably sending EDUs that is not present at the moment (AFAIK).

Closes #57.


  • I agree to release my code and all other changes of this MR under the Apache-2.0 license
This PR adds presence functionality (by default disabled), including configurable intervals and idle/offline timeouts. The only caveat here is that presence events only get batched together when PDUs are sent, somewhat reducing the reliability of presence over federation. This will however require addition of a generalized functionality for reliably sending EDUs that is not present at the moment (AFAIK). Closes #57. <!-- Please describe your changes here --> ----------------------------------------------------------------------------- - [X] I agree to release my code and all other changes of this MR under the Apache-2.0 license
M0dEx commented 2023-05-28 22:10:33 +00:00 (Migrated from gitlab.com)

assigned to @M0dEx

assigned to @M0dEx
M0dEx commented 2023-05-28 22:17:45 +00:00 (Migrated from gitlab.com)

added 1 commit

  • fa3108bc - [#57, #58] feat(presence): do not save services() to a variable

Compare with previous version

added 1 commit <ul><li>fa3108bc - [#57, #58] feat(presence): do not save services() to a variable</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=692924732&start_sha=925e77bd17d091ced4f59260b1ec9e50b0781faa)
M0dEx commented 2023-05-28 22:23:47 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 110e92c7 - [#57, #58] style(database/presence): fix clippy warning

Compare with previous version

added 1 commit <ul><li>110e92c7 - [#57, #58] style(database/presence): fix clippy warning</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=692925453&start_sha=fa3108bcd7fca058d83212b3633e6d0232ffb788)
M0dEx commented 2023-05-28 22:25:31 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 9b58bcd8 - [#57, #58] fix(database/mod): do not clear presence table on startup

Compare with previous version

added 1 commit <ul><li>9b58bcd8 - [#57, #58] fix(database/mod): do not clear presence table on startup</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=692925641&start_sha=110e92c717ccb18dbcff3dc78cca03de8365cd68)
M0dEx commented 2023-05-28 22:26:30 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 843a6a77 - [#57, #58] style(database/presence): reformat with cargo fmt

Compare with previous version

added 1 commit <ul><li>843a6a77 - [#57, #58] style(database/presence): reformat with cargo fmt</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=692925826&start_sha=9b58bcd80f5f854c09c78fb27de1b09d46811232)
M0dEx commented 2023-05-29 09:41:24 +00:00 (Migrated from gitlab.com)

added 2 commits

  • ae5b5200 - [#57, #58] feat(presence): add purge timeout
  • 8000bdce - [#57, #58] feat(presence): handle offline state better

Compare with previous version

added 2 commits <ul><li>ae5b5200 - [#57, #58] feat(presence): add purge timeout</li><li>8000bdce - [#57, #58] feat(presence): handle offline state better</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=693213007&start_sha=843a6a77c8ca8ee9ba1d1318dc2e60c870f39005)
M0dEx commented 2023-05-29 11:56:23 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 689e8451 - [#57, #58] feat(presence): only use curently active if state != Offline

Compare with previous version

added 1 commit <ul><li>689e8451 - [#57, #58] feat(presence): only use curently active if state != Offline</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=693331771&start_sha=8000bdce80e7f14fabb8af01dea10ea5382ae99c)
M0dEx commented 2023-05-29 12:09:42 +00:00 (Migrated from gitlab.com)

This might have to be re-done with timers, as it currently takes a relatively long time (150 ms) even for a relatively low-load server (joined in rooms with ~50k people total).

This might have to be re-done with timers, as it currently takes a relatively long time (150 ms) even for a relatively low-load server (joined in rooms with ~50k people total).
M0dEx commented 2023-05-29 21:18:24 +00:00 (Migrated from gitlab.com)

changed this line in version 8 of the diff

changed this line in [version 8 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=693656802&start_sha=689e8451cc81be938641dcc3c36f11070af6f098#20519201e51c34d69bb5323abfcb642a907e9083_325_305)
M0dEx commented 2023-05-29 21:18:25 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 7ba2bdf7 - [#57, #58] feat(presence): re-do presence maintenance task using timers

Compare with previous version

added 1 commit <ul><li>7ba2bdf7 - [#57, #58] feat(presence): re-do presence maintenance task using timers</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=693656802&start_sha=689e8451cc81be938641dcc3c36f11070af6f098)
M0dEx commented 2023-05-29 21:23:01 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 1cab2d93 - [#57, #58] style(database/mod): reformat

Compare with previous version

added 1 commit <ul><li>1cab2d93 - [#57, #58] style(database/mod): reformat</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=693658092&start_sha=7ba2bdf78eaa0f66dd7c1dc1488ac00d5cfa01a5)
M0dEx commented 2023-05-29 21:27:22 +00:00 (Migrated from gitlab.com)

added 1 commit

  • d296ec19 - [#57, #58] feat(config): remove presence maintenance interval

Compare with previous version

added 1 commit <ul><li>d296ec19 - [#57, #58] feat(config): remove presence maintenance interval</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=693659374&start_sha=1cab2d9305d35556cf5df21413fa030c287d2f80)
M0dEx commented 2023-05-29 21:28:12 +00:00 (Migrated from gitlab.com)

changed the description

changed the description
M0dEx commented 2023-05-30 11:01:51 +00:00 (Migrated from gitlab.com)

added 1 commit

  • d952f0f7 - fix(presence): check last_active_ago correctly during timer processing

Compare with previous version

added 1 commit <ul><li>d952f0f7 - fix(presence): check last_active_ago correctly during timer processing</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=694167571&start_sha=d296ec19f190d83ebb7c041bd5e012b088a14277)
M0dEx commented 2023-05-30 11:14:02 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 7dbea584 - feat(presence): improve presence last_active_ago logic

Compare with previous version

added 1 commit <ul><li>7dbea584 - feat(presence): improve presence last_active_ago logic</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=694185330&start_sha=d952f0f7642c2e39dbe5c5102d6b4ec13b8ddf53)
M0dEx commented 2023-05-30 11:14:32 +00:00 (Migrated from gitlab.com)

added 1 commit

  • fdf762b1 - style(presence): reformat

Compare with previous version

added 1 commit <ul><li>fdf762b1 - style(presence): reformat</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=694185959&start_sha=7dbea584f7646ec3dad4649ae8294bc758329b66)
M0dEx commented 2023-06-01 19:20:50 +00:00 (Migrated from gitlab.com)

The performance looks ok with timers.

I have been running it for a couple of days now and presence seems to be working great, with no noticeable slowdown (or higher CPU/memory consumption).

The performance looks ok with timers. I have been running it for a couple of days now and presence seems to be working great, with no noticeable slowdown (or higher CPU/memory consumption).
M0dEx commented 2023-06-01 19:44:48 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 921b7c51 - feat(sending): move presence before read receipts and use iterators

Compare with previous version

added 1 commit <ul><li>921b7c51 - feat(sending): move presence before read receipts and use iterators</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=697447742&start_sha=fdf762b1c74a1c34f02310760eb36f6bd061ae59)
M0dEx commented 2023-06-01 19:46:34 +00:00 (Migrated from gitlab.com)

added 1 commit

  • c305734f - style(sending): remove unnecessary type annotation

Compare with previous version

added 1 commit <ul><li>c305734f - style(sending): remove unnecessary type annotation</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=697449101&start_sha=921b7c515d2682e0aed11f460ac6913280caafb2)
M0dEx commented 2023-06-01 19:48:37 +00:00 (Migrated from gitlab.com)

If we clear presence data on startup, it would probably make sense to make this a memory-only map.

With a HashMap, we would lose access to useful methods like scan_prefix, though.

If we clear presence data on startup, it would probably make sense to make this a memory-only map. With a HashMap, we would lose access to useful methods like `scan_prefix`, though.
M0dEx commented 2023-06-06 08:15:23 +00:00 (Migrated from gitlab.com)

I do not like this part, but I could not think of anything better.

Ideas on how to prettify this/do this better are welcome.

I do not like this part, but I could not think of anything better. Ideas on how to prettify this/do this better are welcome.
M0dEx commented 2023-06-06 08:15:23 +00:00 (Migrated from gitlab.com)

Document these options in the example config.

Document these options in the example config.
M0dEx commented 2023-06-06 08:15:23 +00:00 (Migrated from gitlab.com)

I do not like this, but it is much faster than serde_json.

If we made presence a memory-only map, we would not need this.

I do not like this, but it is much faster than `serde_json`. If we made presence a memory-only map, we would not need this.
M0dEx commented 2023-06-06 08:15:24 +00:00 (Migrated from gitlab.com)

I am worried about the length and readability of this method.

Maybe split it up?

I am worried about the length and readability of this method. Maybe split it up?
M0dEx commented 2023-06-06 08:15:24 +00:00 (Migrated from gitlab.com)

This is duplicated across ping_presence and set_presence, maybe put it into a separate function/method/macro?

This is duplicated across `ping_presence` and `set_presence`, maybe put it into a separate function/method/macro?
M0dEx commented 2023-06-06 08:15:24 +00:00 (Migrated from gitlab.com)

This is duplicated across multiple methods in this file, maybe make it a function/method/macro?

This is duplicated across multiple methods in this file, maybe make it a function/method/macro?
timokoesters commented 2023-06-06 11:09:20 +00:00 (Migrated from gitlab.com)
new = new.or(old) should work: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3596441037b0e7234aa440e3e261e2ba
M0dEx commented 2023-06-06 11:37:58 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 89bad8aa - style(sync): use .or() when collecting presence updates

Compare with previous version

added 1 commit <ul><li>89bad8aa - style(sync): use .or() when collecting presence updates</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=700629213&start_sha=c305734fd30af723b9c59715b8c7f1342bdffb3e)
M0dEx commented 2023-06-06 11:38:42 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 662e3ed4 - style(sync): reformat

Compare with previous version

added 1 commit <ul><li>662e3ed4 - style(sync): reformat</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=700630023&start_sha=89bad8aa203ab7e603730df718a6d8016ef1bc89)
timokoesters commented 2023-06-06 12:55:50 +00:00 (Migrated from gitlab.com)

I think the values should be more nuanced:
allow_incoming_presence (for federation)
allow_outgoing_presence (for federation)
allow_local_presence (without federation)

I think the values should be more nuanced: allow_incoming_presence (for federation) allow_outgoing_presence (for federation) allow_local_presence (without federation)
timokoesters commented 2023-06-06 12:55:50 +00:00 (Migrated from gitlab.com)

Maybe make sure the time is not in the future? Or do it inside the set_presence fn

Maybe make sure the time is not in the future? Or do it inside the set_presence fn
timokoesters commented 2023-06-06 12:55:50 +00:00 (Migrated from gitlab.com)

Maybe *_secs would be a bit more intuitive?

Maybe *_secs would be a bit more intuitive?
timokoesters commented 2023-06-06 12:55:50 +00:00 (Migrated from gitlab.com)

Please use to_be_bytes

Please use to_be_bytes
timokoesters commented 2023-06-06 12:55:51 +00:00 (Migrated from gitlab.com)

That's a good idea, BTreeMap should work

That's a good idea, BTreeMap should work
timokoesters commented 2023-06-06 12:55:51 +00:00 (Migrated from gitlab.com)

This is fine.

This is fine.
timokoesters commented 2023-06-06 12:55:51 +00:00 (Migrated from gitlab.com)

At some point I want to improve the key value database by introducing another level of abstraction, but for now it's fine

At some point I want to improve the key value database by introducing another level of abstraction, but for now it's fine
timokoesters commented 2023-06-06 12:55:51 +00:00 (Migrated from gitlab.com)

Use bad_database instead of BadDatabase so we get more logs

Use bad_database instead of BadDatabase so we get more logs
timokoesters commented 2023-06-06 12:55:51 +00:00 (Migrated from gitlab.com)

I think this would be easier to read as
size_of::<u8>() + 1 .. size_of::<u8>() + 1 + size_of::<u64>(), maybe inline it directly where you use it.

I think this would be easier to read as `size_of::<u8>() + 1 .. size_of::<u8>() + 1 + size_of::<u64>()`, maybe inline it directly where you use it.
timokoesters commented 2023-06-06 12:55:51 +00:00 (Migrated from gitlab.com)

We have utils::u64_from_bytes to make this slightly shorter

We have utils::u64_from_bytes to make this slightly shorter
timokoesters commented 2023-06-06 12:55:51 +00:00 (Migrated from gitlab.com)

What's the reason for the first condition?

What's the reason for the first condition?
timokoesters commented 2023-06-06 12:55:51 +00:00 (Migrated from gitlab.com)

What if it's None? You could write if presence.map(|p| p.state) != Some(new_state)

What if it's None? You could write `if presence.map(|p| p.state) != Some(new_state)`
timokoesters commented 2023-06-06 12:55:52 +00:00 (Migrated from gitlab.com)

Can you rename this to something like start_handler

Can you rename this to something like `start_handler`
timokoesters commented 2023-06-06 12:55:52 +00:00 (Migrated from gitlab.com)

Here's my review so far, I still have to look at the timer code, but the rest is really well done, thanks!

Here's my review so far, I still have to look at the timer code, but the rest is really well done, thanks!
M0dEx commented 2023-06-10 13:36:20 +00:00 (Migrated from gitlab.com)

BTreeMap with range would work great, the problem is with returning an iterator in presence_since.
The BTreeMap has to be read-locked and the read-lock does not live long enough.

Do you have any ideas on how to be able to return an iterator in presence_since?
Or am I just trying to prematurely optimize and it would be just fine to collect and return a Vec instead?

`BTreeMap` with `range` would work great, the problem is with returning an iterator in `presence_since`. The `BTreeMap` has to be read-locked and the read-lock does not live long enough. Do you have any ideas on how to be able to return an iterator in `presence_since`? Or am I just trying to prematurely optimize and it would be just fine to `collect` and return a `Vec` instead?
M0dEx commented 2023-06-11 13:18:34 +00:00 (Migrated from gitlab.com)

changed this line in version 18 of the diff

changed this line in [version 18 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=705063686&start_sha=662e3ed4d6c8292b45a892dc2544ed7023e0e4ff#1b43193daac5c2e5598d8117572ea207bbb469cd_74_77)
M0dEx commented 2023-06-11 13:18:34 +00:00 (Migrated from gitlab.com)

changed this line in version 18 of the diff

changed this line in [version 18 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=705063686&start_sha=662e3ed4d6c8292b45a892dc2544ed7023e0e4ff#20519201e51c34d69bb5323abfcb642a907e9083_83_16)
M0dEx commented 2023-06-11 13:18:35 +00:00 (Migrated from gitlab.com)

changed this line in version 18 of the diff

changed this line in [version 18 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=705063686&start_sha=662e3ed4d6c8292b45a892dc2544ed7023e0e4ff#20519201e51c34d69bb5323abfcb642a907e9083_259_135)
M0dEx commented 2023-06-11 13:18:35 +00:00 (Migrated from gitlab.com)

changed this line in version 18 of the diff

changed this line in [version 18 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=705063686&start_sha=662e3ed4d6c8292b45a892dc2544ed7023e0e4ff#20519201e51c34d69bb5323abfcb642a907e9083_238_117)
M0dEx commented 2023-06-11 13:18:35 +00:00 (Migrated from gitlab.com)

changed this line in version 18 of the diff

changed this line in [version 18 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=705063686&start_sha=662e3ed4d6c8292b45a892dc2544ed7023e0e4ff#20519201e51c34d69bb5323abfcb642a907e9083_79_16)
M0dEx commented 2023-06-11 13:18:35 +00:00 (Migrated from gitlab.com)

changed this line in version 18 of the diff

changed this line in [version 18 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=705063686&start_sha=662e3ed4d6c8292b45a892dc2544ed7023e0e4ff#20519201e51c34d69bb5323abfcb642a907e9083_99_16)
M0dEx commented 2023-06-11 13:18:36 +00:00 (Migrated from gitlab.com)

changed this line in version 18 of the diff

changed this line in [version 18 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=705063686&start_sha=662e3ed4d6c8292b45a892dc2544ed7023e0e4ff#20519201e51c34d69bb5323abfcb642a907e9083_106_16)
M0dEx commented 2023-06-11 13:18:36 +00:00 (Migrated from gitlab.com)

changed this line in version 18 of the diff

changed this line in [version 18 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=705063686&start_sha=662e3ed4d6c8292b45a892dc2544ed7023e0e4ff#20519201e51c34d69bb5323abfcb642a907e9083_121_16)
M0dEx commented 2023-06-11 13:18:36 +00:00 (Migrated from gitlab.com)

added 1 commit

  • ea6f7af9 - feat(presence): use BTreeMap for presence data

Compare with previous version

added 1 commit <ul><li>ea6f7af9 - feat(presence): use BTreeMap for presence data</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=705063686&start_sha=662e3ed4d6c8292b45a892dc2544ed7023e0e4ff)
M0dEx commented 2023-06-25 21:18:33 +00:00 (Migrated from gitlab.com)

added 27 commits

  • ea6f7af9...5f9ca8e4 - 7 commits from branch next
  • 5f9ca8e4...9e9a75e2 - 10 earlier commits
  • 44d29c53 - [#57, #58] style(database/mod): reformat
  • 13257e4f - [#57, #58] feat(config): remove presence maintenance interval
  • 3f8a214f - fix(presence): check last_active_ago correctly during timer processing
  • f146b8b8 - feat(presence): improve presence last_active_ago logic
  • e0408f96 - style(presence): reformat
  • 08c3ae94 - feat(sending): move presence before read receipts and use iterators
  • 0d1bc61c - style(sending): remove unnecessary type annotation
  • 37c1f4e4 - style(sync): use .or() when collecting presence updates
  • 25176204 - style(sync): reformat
  • 326bd146 - feat(presence): use BTreeMap for presence data

Compare with previous version

added 27 commits <ul><li>ea6f7af9...5f9ca8e4 - 7 commits from branch <code>next</code></li><li>5f9ca8e4...9e9a75e2 - 10 earlier commits</li><li>44d29c53 - [#57, #58] style(database/mod): reformat</li><li>13257e4f - [#57, #58] feat(config): remove presence maintenance interval</li><li>3f8a214f - fix(presence): check last_active_ago correctly during timer processing</li><li>f146b8b8 - feat(presence): improve presence last_active_ago logic</li><li>e0408f96 - style(presence): reformat</li><li>08c3ae94 - feat(sending): move presence before read receipts and use iterators</li><li>0d1bc61c - style(sending): remove unnecessary type annotation</li><li>37c1f4e4 - style(sync): use .or() when collecting presence updates</li><li>25176204 - style(sync): reformat</li><li>326bd146 - feat(presence): use BTreeMap for presence data</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=717573328&start_sha=ea6f7af98d4129e90c26b61633c0e529209a03a9)
M0dEx commented 2023-07-03 16:02:17 +00:00 (Migrated from gitlab.com)

added 50 commits

  • 326bd146...833c1505 - 30 commits from branch next
  • 833c1505...73855c44 - 10 earlier commits
  • 26dae1e2 - [#57, #58] style(database/mod): reformat
  • 0bef084f - [#57, #58] feat(config): remove presence maintenance interval
  • 8ed82a78 - fix(presence): check last_active_ago correctly during timer processing
  • 44f6a024 - feat(presence): improve presence last_active_ago logic
  • fdd71b8e - style(presence): reformat
  • f456116b - feat(sending): move presence before read receipts and use iterators
  • 02d91b88 - style(sending): remove unnecessary type annotation
  • 5be936f2 - style(sync): use .or() when collecting presence updates
  • 44b15034 - style(sync): reformat
  • 6b2eae77 - feat(presence): use BTreeMap for presence data

Compare with previous version

added 50 commits <ul><li>326bd146...833c1505 - 30 commits from branch <code>next</code></li><li>833c1505...73855c44 - 10 earlier commits</li><li>26dae1e2 - [#57, #58] style(database/mod): reformat</li><li>0bef084f - [#57, #58] feat(config): remove presence maintenance interval</li><li>8ed82a78 - fix(presence): check last_active_ago correctly during timer processing</li><li>44f6a024 - feat(presence): improve presence last_active_ago logic</li><li>fdd71b8e - style(presence): reformat</li><li>f456116b - feat(sending): move presence before read receipts and use iterators</li><li>02d91b88 - style(sending): remove unnecessary type annotation</li><li>5be936f2 - style(sync): use .or() when collecting presence updates</li><li>44b15034 - style(sync): reformat</li><li>6b2eae77 - feat(presence): use BTreeMap for presence data</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=724292767&start_sha=326bd146e9c420f31e0062215970d2c4be1daa3b)
M0dEx commented 2023-07-03 17:15:46 +00:00 (Migrated from gitlab.com)

changed this line in version 21 of the diff

changed this line in [version 21 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=724342315&start_sha=6b2eae777a6ad2781e7cf5833127467fd7c8a17b#0f9c00522d142334a2ff97ca0764cb1a9dc063ce_88_87)
M0dEx commented 2023-07-03 17:16:09 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 72d1e973 - feat(presence): implement presence functionality

Compare with previous version

added 1 commit <ul><li>72d1e973 - feat(presence): implement presence functionality</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=724342533&start_sha=6b2eae777a6ad2781e7cf5833127467fd7c8a17b)
M0dEx commented 2023-07-03 17:19:10 +00:00 (Migrated from gitlab.com)

last_active_ago will never be in the future, as it is an unsigned integer.

`last_active_ago` will never be in the future, as it is an unsigned integer.
M0dEx commented 2023-07-03 17:20:15 +00:00 (Migrated from gitlab.com)

If it is None, then we do not care about it.

If it is `None`, then we do not care about it.
M0dEx commented 2023-07-03 17:45:26 +00:00 (Migrated from gitlab.com)

added 3 commits

  • 72d1e973...f8a36e75 - 2 commits from branch next
  • c7aa5652 - feat(presence): implement presence functionality

Compare with previous version

added 3 commits <ul><li>72d1e973...f8a36e75 - 2 commits from branch <code>next</code></li><li>c7aa5652 - feat(presence): implement presence functionality</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=724357617&start_sha=72d1e97379787f9dec8ed5b40c2271c2c40be754)
M0dEx commented 2023-07-10 14:42:23 +00:00 (Migrated from gitlab.com)

added 10 commits

  • c7aa5652...ad06d475 - 9 commits from branch next
  • 36f551b1 - feat(presence): implement presence functionality

Compare with previous version

added 10 commits <ul><li>c7aa5652...ad06d475 - 9 commits from branch <code>next</code></li><li>36f551b1 - feat(presence): implement presence functionality</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=729879261&start_sha=c7aa56521e1016c13f75c4eb14a97ceed1728d9e)
M0dEx commented 2023-09-08 11:53:46 +00:00 (Migrated from gitlab.com)

added 89 commits

  • 36f551b1...90fea00d - 88 commits from branch next
  • 1fa52b60 - feat(presence): implement presence functionality

Compare with previous version

added 89 commits <ul><li>36f551b1...90fea00d - 88 commits from branch <code>next</code></li><li>1fa52b60 - feat(presence): implement presence functionality</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=783072135&start_sha=36f551b12e756fc89e352fb784dcae117b8cce46)
M0dEx commented 2023-09-08 12:36:48 +00:00 (Migrated from gitlab.com)

changed this line in version 26 of the diff

changed this line in [version 26 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=783122553&start_sha=1fa52b60e5fc9b232d218dc7d667369308d46d54#312d406453d072f966568e7e9199c628ba9d6882_285_257)
M0dEx commented 2023-09-08 12:36:48 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 2e67c0b0 - feat(presence): add granular allow configuration

Compare with previous version

added 1 commit <ul><li>2e67c0b0 - feat(presence): add granular allow configuration</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=783122553&start_sha=1fa52b60e5fc9b232d218dc7d667369308d46d54)
M0dEx commented 2023-09-08 12:37:38 +00:00 (Migrated from gitlab.com)

Does it add any clarity? Other intervals in the config do not even have the _s suffix.

Does it add any clarity? Other intervals in the config do not even have the `_s` suffix.
M0dEx commented 2023-09-08 12:40:13 +00:00 (Migrated from gitlab.com)

added 2 commits

  • ac7c2643 - feat(presence): implement presence functionality
  • 1e6729a0 - feat(presence): add granular allow configuration

Compare with previous version

added 2 commits <ul><li>ac7c2643 - feat(presence): implement presence functionality</li><li>1e6729a0 - feat(presence): add granular allow configuration</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=783126667&start_sha=2e67c0b052d97e0621689947615d9c98cacf11a6)
M0dEx commented 2023-09-08 12:43:40 +00:00 (Migrated from gitlab.com)

added 2 commits

  • 5c87dd55 - feat(presence): implement presence functionality
  • 415f9c4d - feat(presence): add granular allow configuration

Compare with previous version

added 2 commits <ul><li>5c87dd55 - feat(presence): implement presence functionality</li><li>415f9c4d - feat(presence): add granular allow configuration</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=783130789&start_sha=1e6729a0ad49162a3dbfd9fcc28369d8fa4db488)
M0dEx commented 2023-09-08 12:44:12 +00:00 (Migrated from gitlab.com)

Fixed according to spec:

To reduce the number of presence updates sent to clients the server may include a currently_active boolean field when the presence state is online. When true, the server will not send further updates to the last active time until an update is sent to the client with either a) currently_active set to false or b) a presence state other than online. During this period clients must consider the user to be currently active, irrespective of the last active time.

Fixed according to spec: > To reduce the number of presence updates sent to clients the server may include a currently_active boolean field when the presence state is online. When true, the server will not send further updates to the last active time until an update is sent to the client with either a) currently_active set to false or b) a presence state other than online. During this period clients must consider the user to be currently active, irrespective of the last active time.
M0dEx commented 2023-09-15 13:55:48 +00:00 (Migrated from gitlab.com)

The current status of this MR:

AFAIK, all required functionality from the spec is implemented.

Presence data is stored in the database (someone more familiar with Conduit might devise a more efficient way to save the data, regarding the response times), permanently.

I've been running on this branch for a couple of months and have not experienced any performance degradation or significantly increased resource consumption. I'd love for someone to try this with a bigger server, with a lot of local users.

All threads with initial comments have been resolved with the issues either fixed or refactored.

### The current status of this MR: AFAIK, all required functionality from the spec is implemented. Presence data is stored in the database (someone more familiar with Conduit might devise a more efficient way to save the data, regarding the response times), permanently. I've been running on this branch for a couple of months and have not experienced any performance degradation or significantly increased resource consumption. I'd love for someone to try this with a bigger server, with a lot of local users. All threads with initial comments have been resolved with the issues either fixed or refactored.
M0dEx commented 2023-09-15 13:58:12 +00:00 (Migrated from gitlab.com)

added 9 commits

  • 415f9c4d...3bfdae79 - 7 commits from branch next
  • 4d7030d8 - feat(presence): implement presence functionality
  • 13181ef1 - feat(presence): add granular allow configuration

Compare with previous version

added 9 commits <ul><li>415f9c4d...3bfdae79 - 7 commits from branch <code>next</code></li><li>4d7030d8 - feat(presence): implement presence functionality</li><li>13181ef1 - feat(presence): add granular allow configuration</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=790595129&start_sha=415f9c4d4232cc94160715794961a55aae01549b)
M0dEx commented 2023-09-17 09:46:09 +00:00 (Migrated from gitlab.com)

requested review from @timokoesters

requested review from @timokoesters
girlbossceo commented 2023-12-13 15:11:00 +00:00 (Migrated from gitlab.com)

mentioned in issue #414

mentioned in issue #414
M0dEx commented 2023-12-20 10:28:23 +00:00 (Migrated from gitlab.com)

added 4 commits

  • 13181ef1...30f0871e - 2 commits from branch next
  • b88d5486 - feat(presence): implement presence functionality
  • ff8a1f64 - feat(presence): add granular allow configuration

Compare with previous version

added 4 commits <ul><li>13181ef1...30f0871e - 2 commits from branch <code>next</code></li><li>b88d5486 - feat(presence): implement presence functionality</li><li>ff8a1f64 - feat(presence): add granular allow configuration</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=877619451&start_sha=13181ef16fd988d1ea06c6453968a8e836658781)
lnicola commented 2023-12-20 10:36:26 +00:00 (Migrated from gitlab.com)

Nit:

            "Presence state for this user was not found",
Nit: ```suggestion:-0+0 "Presence state for this user was not found", ```
M0dEx commented 2023-12-20 10:41:52 +00:00 (Migrated from gitlab.com)

changed this line in version 31 of the diff

changed this line in [version 31 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=877638060&start_sha=ff8a1f6469ccbc1d1f1687e10590667a8eb8d3bd#e72f4e14774e775d11cee027e54c1596abd9620a_89_89)
M0dEx commented 2023-12-20 10:41:53 +00:00 (Migrated from gitlab.com)

added 2 commits

  • 71d911f3 - feat(presence): implement presence functionality
  • 4ffcf8f2 - feat(presence): add granular allow configuration

Compare with previous version

added 2 commits <ul><li>71d911f3 - feat(presence): implement presence functionality</li><li>4ffcf8f2 - feat(presence): add granular allow configuration</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=877638060&start_sha=ff8a1f6469ccbc1d1f1687e10590667a8eb8d3bd)
M0dEx commented 2023-12-20 11:05:04 +00:00 (Migrated from gitlab.com)

added 1 commit

  • c83e89d6 - style(presence): use flat_map instead of matching Results in filter

Compare with previous version

added 1 commit <ul><li>c83e89d6 - style(presence): use flat_map instead of matching Results in filter</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=877667300&start_sha=4ffcf8f284d9011a09bc71c85fd93e1dd332a1c8)
lnicola commented 2023-12-20 11:05:25 +00:00 (Migrated from gitlab.com)

Not really familiar with Conduit, but should ping_presence errors be propagated to the caller or simply dropped? Does it even make a difference in practice?

Not really familiar with Conduit, but should `ping_presence` errors be propagated to the caller or simply dropped? Does it even make a difference in practice?
lnicola commented 2023-12-20 11:09:39 +00:00 (Migrated from gitlab.com)

Besides the unfortunate clones, is this right? It seems to keep the old values unless they're None, so Some("foo") and Some("bar") yields Some("foo").

Besides the unfortunate `clone`s, is this right? It seems to keep the old values unless they're `None`, so `Some("foo")` and `Some("bar")` yields `Some("foo")`.
M0dEx commented 2023-12-20 12:25:32 +00:00 (Migrated from gitlab.com)

I could replace the clone() with take(), that should be without any copying or allocations AFAIK.

I could replace the `clone()` with `take()`, that should be without any copying or allocations AFAIK.
lnicola commented 2023-12-20 12:30:04 +00:00 (Migrated from gitlab.com)

Yeah, but it would have to be curr_content.status_msg = new_content.status_msg.or_else(|| curr_content.status_msg.take()); (or simply or, whatever).

You're doing it in the wrong direction right now.

Yeah, but it would have to be `curr_content.status_msg = new_content.status_msg.or_else(|| curr_content.status_msg.take());` (or simply `or`, whatever). You're doing it in the wrong direction right now.
M0dEx commented 2023-12-20 12:32:18 +00:00 (Migrated from gitlab.com)

I see, you're right.

I see, you're right.
M0dEx commented 2023-12-20 12:34:54 +00:00 (Migrated from gitlab.com)

changed this line in version 33 of the diff

changed this line in [version 33 of the diff](/famedly/conduit/-/merge_requests/476/diffs?diff_id=877773754&start_sha=c83e89d6cda0c037dc01f147827bcc3031e29c57#312d406453d072f966568e7e9199c628ba9d6882_556_556)
M0dEx commented 2023-12-20 12:34:54 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 67efee9a - fix(sync): correctly update presence properties

Compare with previous version

added 1 commit <ul><li>67efee9a - fix(sync): correctly update presence properties</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=877773754&start_sha=c83e89d6cda0c037dc01f147827bcc3031e29c57)
CobaltCause commented 2023-12-24 05:03:30 +00:00 (Migrated from gitlab.com)

mentioned in merge request !565

mentioned in merge request !565
M0dEx commented 2023-12-24 15:50:29 +00:00 (Migrated from gitlab.com)

added 8 commits

  • 67efee9a...ca621972 - 4 commits from branch next
  • ed42bcce - feat(presence): implement presence functionality
  • 100c8b39 - feat(presence): add granular allow configuration
  • 76a90981 - style(presence): use flat_map instead of matching Results in filter
  • 8690552b - fix(sync): correctly update presence properties

Compare with previous version

added 8 commits <ul><li>67efee9a...ca621972 - 4 commits from branch <code>next</code></li><li>ed42bcce - feat(presence): implement presence functionality</li><li>100c8b39 - feat(presence): add granular allow configuration</li><li>76a90981 - style(presence): use flat_map instead of matching Results in filter</li><li>8690552b - fix(sync): correctly update presence properties</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/476/diffs?diff_id=880553803&start_sha=67efee9a5fedab5a1bd1158a0dbe9e6746a6f8ff)
timokoesters commented 2024-01-28 19:35:13 +00:00 (Migrated from gitlab.com)

mentioned in issue #412

mentioned in issue #412
This pull request has changes conflicting with the target branch.
  • src/api/client_server/sync.rs
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin presence:presence
git checkout presence

Merge

Merge the changes and update on Forgejo.
git checkout next
git merge --no-ff presence
git checkout next
git merge --ff-only presence
git checkout presence
git rebase next
git checkout next
git merge --no-ff presence
git checkout next
git merge --squash presence
git checkout next
git merge --ff-only presence
git checkout next
git merge presence
git push origin next
Sign in to join this conversation.
No reviewers
No labels
Android
CS::needs customer feedback
CS::needs follow up
CS::needs on prem installation
CS::waiting
Chrome
Design:: Ready
Design:: in progress
Design::UX
E2EE
Edge
Firefox
GDPR
Iteration 13 IM
Linux
MacOS
Need::Discussion
Need::Steps to reproduce
Need::Upstream fix
Needs:: Planning
Needs::Dev-Team
Needs::More information
Needs::Priority
Needs::Product
Needs::Refinement
Needs::Severity
Priority::1-Critical
Priority::2-Max
Priority::3-Impending
Priority::4-High
Priority::5-Medium
Priority::6-Low
Priority::7-None
Progress::Backlog
Progress::Review
Progress::Started
Progress::Testing
Progress::Triage
Progress::Waiting
Reporter::Sentry
Safari
Target::Community
Target::Customer
Target::Internal
Target::PoC
Target::Security
Team:Customer-Success
Team:Design
Team:Infrastructure
Team:Instant-Messaging
Team:Product
Team:Workflows
Type::Bug
Type::Design
Type::Documentation
Type::Feature
Type::Improvement
Type::Support
Type::Tests
Windows
blocked
blocked-by-spec
cla-signed
conduit
contribution::advanced
contribution::easy
contribution::help needed
from::review
iOS
p::ti-tenant
performance
product::triage
proposal
refactor
release-blocker
s: dart_openapi_codegen
s::Famedly-Patient
s::Org-Directory
s::Passport-Generator
s::Requeuest
s:CRM
s:Famedly-App
s:Famedly-Web
s:Fhiroxide
s:Fhiroxide-cli
s:Fhiroxide-client
s:Fhirs
s:Hedwig
s:LISA
s:Matrix-Dart-SDK
s:Role-Manager
s:Synapse
s:User-Directory
s:WFS-Matrix
s:Workflow Engine
s:dtls
s:famedly-error
s:fcm-shared-isolate
s:matrix-api-lite
s:multiple-tab-detector
s:native-imaging
severity::1
severity::2
severity::3
severity::4
technical-debt
voip
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Matthias/conduit#901
No description provided.