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

Closed
M0dEx wants to merge 26 commits from presence into next
M0dEx commented 2022-11-18 16:22:47 +00:00 (Migrated from gitlab.com)

This MR adds the capability to handle presence-over-federation EDUs, which allows users to see status of users from other servers.

Linked issues

  • Closes #57
  • Partially completes #58

TODO

  • Receive presence updates over federation
  • (Unreliably) Send presence updates over federation
  • Keeping presence up-to-date (timed tasks?)
  • Database cleanup

  • I agree to release my code and all other changes of this MR under the Apache-2.0 license
This MR adds the capability to handle presence-over-federation EDUs, which allows users to see status of users from other servers. ### Linked issues - Closes #57 - Partially completes #58 ### TODO - [x] Receive presence updates over federation - [x] (Unreliably) Send presence updates over federation - [x] Keeping presence up-to-date (timed tasks?) - [x] Database cleanup ----------------------------------------------------------------------------- - [X] I agree to release my code and all other changes of this MR under the Apache-2.0 license
M0dEx commented 2022-11-18 16:22:47 +00:00 (Migrated from gitlab.com)

assigned to @M0dEx

assigned to @M0dEx
M0dEx commented 2022-11-18 16:22:54 +00:00 (Migrated from gitlab.com)

marked the checklist item (Unreliably) Send presence updates over federation as incomplete

marked the checklist item **(Unreliably) Send presence updates over federation** as incomplete
M0dEx commented 2022-11-18 18:23:27 +00:00 (Migrated from gitlab.com)

marked the checklist item Database cleanup as incomplete

marked the checklist item **Database cleanup** as incomplete
M0dEx commented 2022-11-18 18:23:30 +00:00 (Migrated from gitlab.com)

marked the checklist item Keeping presence up-to-date (timed tasks?) as incomplete

marked the checklist item **Keeping presence up-to-date (timed tasks?)** as incomplete
M0dEx commented 2022-11-18 21:33:06 +00:00 (Migrated from gitlab.com)

added 1 commit

  • f6bde5d8 - fix(presence): fix issues found when testing

Compare with previous version

added 1 commit <ul><li>f6bde5d8 - fix(presence): fix issues found when testing</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=537270687&start_sha=ab07c938c6031b7de0b08d9f533525266de6af6b)
M0dEx commented 2022-11-18 21:39:12 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 2de4f2f8 - style(presence): reformat with cargo

Compare with previous version

added 1 commit <ul><li>2de4f2f8 - style(presence): reformat with cargo</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=537273243&start_sha=f6bde5d8b4084d10b52b53544fa4e6664a622e97)
M0dEx commented 2022-11-18 22:56:01 +00:00 (Migrated from gitlab.com)

marked the checklist item Keeping presence up-to-date (timed tasks?) as completed

marked the checklist item **Keeping presence up-to-date (timed tasks?)** as completed
M0dEx commented 2022-11-18 22:56:23 +00:00 (Migrated from gitlab.com)

changed the description

changed the description
M0dEx commented 2022-11-19 21:02:41 +00:00 (Migrated from gitlab.com)

added 1 commit

  • d98a69f7 - feat(presence): send presence events for own users unreliably

Compare with previous version

added 1 commit <ul><li>d98a69f7 - feat(presence): send presence events for own users unreliably</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=537469473&start_sha=2de4f2f837e7719ff70b6bd26d29750ffe6b2347)
M0dEx commented 2022-11-19 21:03:15 +00:00 (Migrated from gitlab.com)

added 1 commit

  • bc245e56 - style(presence): reformat with cargo fmt

Compare with previous version

added 1 commit <ul><li>bc245e56 - style(presence): reformat with cargo fmt</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=537469539&start_sha=d98a69f7caacfad8604f1492abbd9c7886538f46)
M0dEx commented 2022-11-19 21:24:57 +00:00 (Migrated from gitlab.com)

marked the checklist item (Unreliably) Send presence updates over federation as completed

marked the checklist item **(Unreliably) Send presence updates over federation** as completed
M0dEx commented 2022-11-19 23:12:24 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 3cb68305 - style(config): remove useless cast

Compare with previous version

added 1 commit <ul><li>3cb68305 - style(config): remove useless cast</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=537481851&start_sha=bc245e566aed91b5e85d4081e8ce4203d1f0aa39)
M0dEx commented 2022-11-21 20:46:25 +00:00 (Migrated from gitlab.com)

added 2 commits

  • 88147cb7 - feat(presence): start work on cleanup task
  • 92d6cb54 - feat(presence): finish presence cleanup task

Compare with previous version

added 2 commits <ul><li>88147cb7 - feat(presence): start work on cleanup task</li><li>92d6cb54 - feat(presence): finish presence cleanup task</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=538618612&start_sha=3cb68305e7e57783e283219a5904ede335cd88f4)
M0dEx commented 2022-11-21 20:50:34 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 8c8f5c7c - style(presence): reformat

Compare with previous version

added 1 commit <ul><li>8c8f5c7c - style(presence): reformat</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=538621820&start_sha=92d6cb541926c72dd90071d28bfb968c000cb9c6)
M0dEx commented 2022-11-21 21:03:55 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 21e04bd6 - fix(presence): move sleep in presence cleanup

Compare with previous version

added 1 commit <ul><li>21e04bd6 - fix(presence): move sleep in presence cleanup</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=538631954&start_sha=8c8f5c7cc529229b516e3b6102302618298d6112)
M0dEx commented 2022-11-21 21:13:20 +00:00 (Migrated from gitlab.com)

added 1 commit

  • c930cbfa - fix(presence): don't cause panic

Compare with previous version

added 1 commit <ul><li>c930cbfa - fix(presence): don&#39;t cause panic</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=538638414&start_sha=21e04bd6c6e7decda67464ef5dd007bb887cf41a)
M0dEx commented 2022-11-21 21:39:28 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 364184d1 - fix(presence): allow services to start before running tasks

Compare with previous version

added 1 commit <ul><li>364184d1 - fix(presence): allow services to start before running tasks</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=538654063&start_sha=c930cbfa6dbec4c4c900ad7ec71c0724a4dbd8b2)
M0dEx commented 2022-11-21 21:49:03 +00:00 (Migrated from gitlab.com)

added 1 commit

  • efc873f1 - feat(presence): remove old presence updates

Compare with previous version

added 1 commit <ul><li>efc873f1 - feat(presence): remove old presence updates</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=538659101&start_sha=364184d13d2cea36f9ec193fb39816cd2637e550)
M0dEx commented 2022-11-21 21:57:03 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 0f217416 - fix(presence): fix configuration values for presence status

Compare with previous version

added 1 commit <ul><li>0f217416 - fix(presence): fix configuration values for presence status</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=538663444&start_sha=efc873f1534173717ca5cfe2718697b026523964)
M0dEx commented 2022-11-22 13:16:44 +00:00 (Migrated from gitlab.com)

marked the checklist item Database cleanup as completed

marked the checklist item **Database cleanup** as completed
M0dEx commented 2022-11-22 13:17:14 +00:00 (Migrated from gitlab.com)

I'd welcome reviews and some tips on how to make this a bit cleaner.

I'd welcome reviews and some tips on how to make this a bit cleaner.
M0dEx commented 2022-11-22 13:17:30 +00:00 (Migrated from gitlab.com)

requested review from @timokoesters

requested review from @timokoesters
M0dEx commented 2022-11-22 13:17:42 +00:00 (Migrated from gitlab.com)

marked this merge request as ready

marked this merge request as **ready**
Orhideous commented 2022-11-22 15:36:47 +00:00 (Migrated from gitlab.com)
        Ok(get_presence::v3::Response::new(PresenceState::Offline))
```suggestion:-5+0 Ok(get_presence::v3::Response::new(PresenceState::Offline)) ```
Orhideous commented 2022-11-22 15:36:47 +00:00 (Migrated from gitlab.com)
        Ok(Box::new(
            self.roomuserid_presenceevent
                .scan_prefix(room_id.as_bytes().to_vec())
                .filter_map(move |(roomuserid_bytes, presence_bytes)| {
                    let user_id_bytes = roomuserid_bytes.split(|byte| *byte == 0xff).last()?;
                    let user_id: OwnedUserId = UserId::parse(
                        utils::string_from_bytes(&user_id_bytes)
                            .expect("UserID bytes are a valid string"),
                    )
                    .expect("UserID bytes from database are a valid UserID");
                    let timestamp = user_timestamp.get(&user_id)?;
                    let presence_event = parse_presence_event(&presence_bytes, *timestamp)
                        .expect("PresenceEvent bytes from database are a valid PresenceEvent");
                    Some((user_id, presence_event))
                }),
        ))

Those two filter_map can be joined

```suggestion:-28+0 Ok(Box::new( self.roomuserid_presenceevent .scan_prefix(room_id.as_bytes().to_vec()) .filter_map(move |(roomuserid_bytes, presence_bytes)| { let user_id_bytes = roomuserid_bytes.split(|byte| *byte == 0xff).last()?; let user_id: OwnedUserId = UserId::parse( utils::string_from_bytes(&user_id_bytes) .expect("UserID bytes are a valid string"), ) .expect("UserID bytes from database are a valid UserID"); let timestamp = user_timestamp.get(&user_id)?; let presence_event = parse_presence_event(&presence_bytes, *timestamp) .expect("PresenceEvent bytes from database are a valid PresenceEvent"); Some((user_id, presence_event)) }), )) ``` Those two `filter_map` can be joined
Orhideous commented 2022-11-22 15:36:47 +00:00 (Migrated from gitlab.com)

Just minor suggestions

Just minor suggestions
M0dEx commented 2022-11-22 17:37:11 +00:00 (Migrated from gitlab.com)

changed this line in version 14 of the diff

changed this line in [version 14 of the diff](/famedly/conduit/-/merge_requests/418/diffs?diff_id=539649283&start_sha=0f217416ffbb79d60291b1ae069b0e7e4ed75b71#e72f4e14774e775d11cee027e54c1596abd9620a_87_82)
M0dEx commented 2022-11-22 17:37:12 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 7ab7b9b4 - style(presence): code cleanup

Compare with previous version

added 1 commit <ul><li>7ab7b9b4 - style(presence): code cleanup</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=539649283&start_sha=0f217416ffbb79d60291b1ae069b0e7e4ed75b71)
M0dEx commented 2022-11-22 17:37:21 +00:00 (Migrated from gitlab.com)

resolved all threads

resolved all threads
M0dEx commented 2022-11-22 17:37:45 +00:00 (Migrated from gitlab.com)

Thanks for those!

Thanks for those!
M0dEx commented 2022-11-27 16:52:49 +00:00 (Migrated from gitlab.com)

added 30 commits

  • 7ab7b9b4...249960b1 - 11 commits from branch next
  • cd5a83d4 - feat(presence): start presence timeout implementation
  • deeffde6 - feat(presence): restructure database trees for presence
  • f956e727 - feat(presence): refactor presence_since
  • e7348621 - feat(presence): implement most features for PoC
  • b6541d20 - fix(presence): fix default timeout values
  • b18b228c - fix(presence): fix issues found when testing
  • 42b27f2f - style(presence): reformat with cargo
  • 8efcec62 - feat(presence): send presence events for own users unreliably
  • 5bdc0312 - style(presence): reformat with cargo fmt
  • 5e4e4d00 - style(config): remove useless cast
  • f9d10e8f - feat(presence): start work on cleanup task
  • 8d161c6a - feat(presence): finish presence cleanup task
  • 230f09f8 - style(presence): reformat
  • 63ac118d - fix(presence): move sleep in presence cleanup
  • 77b555f2 - fix(presence): don't cause panic
  • dd85316b - fix(presence): allow services to start before running tasks
  • 4d22cb50 - feat(presence): remove old presence updates
  • 2eb5907d - fix(presence): fix configuration values for presence status
  • 50252678 - style(presence): code cleanup

Compare with previous version

added 30 commits <ul><li>7ab7b9b4...249960b1 - 11 commits from branch <code>next</code></li><li>cd5a83d4 - feat(presence): start presence timeout implementation</li><li>deeffde6 - feat(presence): restructure database trees for presence</li><li>f956e727 - feat(presence): refactor presence_since</li><li>e7348621 - feat(presence): implement most features for PoC</li><li>b6541d20 - fix(presence): fix default timeout values</li><li>b18b228c - fix(presence): fix issues found when testing</li><li>42b27f2f - style(presence): reformat with cargo</li><li>8efcec62 - feat(presence): send presence events for own users unreliably</li><li>5bdc0312 - style(presence): reformat with cargo fmt</li><li>5e4e4d00 - style(config): remove useless cast</li><li>f9d10e8f - feat(presence): start work on cleanup task</li><li>8d161c6a - feat(presence): finish presence cleanup task</li><li>230f09f8 - style(presence): reformat</li><li>63ac118d - fix(presence): move sleep in presence cleanup</li><li>77b555f2 - fix(presence): don&#39;t cause panic</li><li>dd85316b - fix(presence): allow services to start before running tasks</li><li>4d22cb50 - feat(presence): remove old presence updates</li><li>2eb5907d - fix(presence): fix configuration values for presence status</li><li>50252678 - style(presence): code cleanup</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=542972440&start_sha=7ab7b9b4c3ff39d96ec81a348fd6ef909f9eb7c1)
timokoesters commented 2022-11-27 17:19:07 +00:00 (Migrated from gitlab.com)

added 2 commits

  • f269a15c - chore: code cleanup / cargo clippy
  • a6d5bfe3 - Merge branch 'Nyaaori/cleanup-presence' into 'presence'

Compare with previous version

added 2 commits <ul><li>f269a15c - chore: code cleanup / cargo clippy</li><li>a6d5bfe3 - Merge branch &#39;Nyaaori/cleanup-presence&#39; into &#39;presence&#39;</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=542975536&start_sha=502526789d27c0c1d460b8e6e3eb0b88a3013ae0)
girlbossceo commented 2022-12-06 19:37:24 +00:00 (Migrated from gitlab.com)

Is there a way to disable/enable this in the config if the user so chooses to? I'm not seeing a way in this MR.

Is there a way to disable/enable this in the config if the user so chooses to? I'm not seeing a way in this MR.
M0dEx commented 2022-12-07 17:27:30 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 1f698718 - feat(presence): add configuration option to disable presence

Compare with previous version

added 1 commit <ul><li>1f698718 - feat(presence): add configuration option to disable presence</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=551860047&start_sha=a6d5bfe35fff2e044c407e840a6ee5966c171e3e)
M0dEx commented 2022-12-07 17:28:32 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 70652fe0 - style(presence): reformat with cargo fmt

Compare with previous version

added 1 commit <ul><li>70652fe0 - style(presence): reformat with cargo fmt</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=551861666&start_sha=1f698718a04613319a19299840c28d3a09e75f23)
M0dEx commented 2022-12-07 17:30:25 +00:00 (Migrated from gitlab.com)

Added in the latest commits.

Added in the latest commits.
M0dEx commented 2022-12-07 17:30:26 +00:00 (Migrated from gitlab.com)

resolved all threads

resolved all threads
M0dEx commented 2022-12-07 17:36:51 +00:00 (Migrated from gitlab.com)

added 1 commit

  • b4356917 - style(presence): make type alias for presence_since iter

Compare with previous version

added 1 commit <ul><li>b4356917 - style(presence): make type alias for presence_since iter</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=551868959&start_sha=70652fe00cec5ff940b89b4474985094ccae470c)
M0dEx commented 2022-12-07 17:39:37 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 46676267 - config(example): add allow_presence

Compare with previous version

added 1 commit <ul><li>46676267 - config(example): add allow_presence</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=551871274&start_sha=b4356917ebe8eb4a6478a7aad7df29dc70a655e3)
M0dEx commented 2022-12-07 17:43:32 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 8257d044 - fix(presence): check for allow_presence only after services are available

Compare with previous version

added 1 commit <ul><li>8257d044 - fix(presence): check for allow_presence only after services are available</li></ul> [Compare with previous version](/famedly/conduit/-/merge_requests/418/diffs?diff_id=551874271&start_sha=46676267df6fd0c6d6fa90b705f4a80714839d1c)
timokoesters commented 2022-12-07 17:55:41 +00:00 (Migrated from gitlab.com)

I think this comment is outdated?

I think this comment is outdated?
timokoesters commented 2022-12-07 17:55:41 +00:00 (Migrated from gitlab.com)

It looks to me like update_count and update_timestamp always have the same value, would it make sense to combine them into one parameter called active (meaning the user actively initiated this change)?

It looks to me like update_count and update_timestamp always have the same value, would it make sense to combine them into one parameter called `active` (meaning the user actively initiated this change)?
timokoesters commented 2022-12-07 17:55:42 +00:00 (Migrated from gitlab.com)

Can you give a short explanation on what this code does? What does it mean to compare the curr and prev presence state?

Can you give a short explanation on what this code does? What does it mean to compare the curr and prev presence state?
M0dEx commented 2022-12-07 18:09:10 +00:00 (Migrated from gitlab.com)

No it is not - the reason I put it there is that we still do not have any component to reliably send events like these over federation.

No it is not - the reason I put it there is that we still do not have any component to reliably send events like these over federation.
M0dEx commented 2022-12-07 18:12:57 +00:00 (Migrated from gitlab.com)

What I am trying to do there is to update the DB count through ping_presence only if the presence state changes. If the presence states are equal, nothing has changed and we do not need to needlessly update the count.

What I am trying to do there is to update the DB count through ping_presence only if the presence state changes. If the presence states are equal, nothing has changed and we do not need to needlessly update the count.
M0dEx commented 2022-12-07 18:13:54 +00:00 (Migrated from gitlab.com)

Unfortunately, they do not always have the same value. For example, in presence_maintain, update_count is true while update_timestamp is false.

Unfortunately, they do not always have the same value. For example, in `presence_maintain`, `update_count` is `true` while `update_timestamp` is `false`.
timokoesters commented 2022-12-16 08:34:56 +00:00 (Migrated from gitlab.com)

Sorry for the delay, the reason why I haven't merged it yet is because the code uses timestamps to calculate the presence state and that will make it difficult to add manual presence changes (see /sync set_presence). It's probably possible to hack it into this code, but there is probably a cleaner solution, what do you think?

Sorry for the delay, the reason why I haven't merged it yet is because the code uses timestamps to calculate the presence state and that will make it difficult to add manual presence changes (see /sync set_presence). It's probably possible to hack it into this code, but there is probably a cleaner solution, what do you think?
M0dEx commented 2022-12-17 18:34:12 +00:00 (Migrated from gitlab.com)

Ah, I see.

What if the code only changed the state in presence_maintain if the state degraded (eg. online -> idle and idle -> offline)?
That seems like a more elegant solution that would respect set_presence /sync.

Ah, I see. What if the code only changed the state in presence_maintain if the state degraded (eg. online -> idle and idle -> offline)? That seems like a more elegant solution that would respect set_presence /sync.
M0dEx commented 2022-12-17 18:50:27 +00:00 (Migrated from gitlab.com)

It will, however, require a bit of a redesign.

It will, however, require a bit of a redesign.
timokoesters commented 2022-12-17 18:53:01 +00:00 (Migrated from gitlab.com)

That sounds better, yes. Saving the current presence state and the timestamp of the last (manual?) presence change should work, right? (Only one timestamp should be enough)

That sounds better, yes. Saving the current presence state and the timestamp of the last (manual?) presence change should work, right? (Only one timestamp should be enough)
M0dEx commented 2022-12-17 18:59:10 +00:00 (Migrated from gitlab.com)

I will have to rethink the entire presence code, probably.

I will have to rethink the entire presence code, probably.
timokoesters commented 2023-01-09 15:48:03 +00:00 (Migrated from gitlab.com)

marked this merge request as draft

marked this merge request as **draft**

Pull request closed

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#843
No description provided.