fix: Allow device ID to be optional when iterating over /messages #757
No reviewers
Labels
No labels
Bug
Cherry-picking
Dependencies
Documentation
Duplicate
Enhancement
Good first issue
Help wanted
Inherited
Invalid
Matrix/Administration
Matrix/Appservices
Matrix/Auth
Matrix/Client
Matrix/Federation
Matrix/MSC
Matrix/Media
Meta
Performance
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Priority
Unknown
Security
Wont fix
no-priority
old/blocked
old/ci/cd
old/core-matrix
old/database
old/github_actions
old/high-priority
old/low-priority
old/medium-priority
old/question
old/rocksdb
old/rust
packaging
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: continuwuation/continuwuity#757
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "nex/fix-appservices-history"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This fixes #738 by allowing the device ID to be optional, which (as far as I can tell) is only the case for appservices.
I've tested this and I can check history using the Meowlnir appservice:
Looks mostly good to me, just some code style stuff to fix
@ -69,3 +64,2 @@
debug_assert!(IGNORED_MESSAGE_TYPES.is_sorted(), "IGNORED_MESSAGE_TYPES is not sorted");
let sender = body.sender();
let (sender_user, sender_device) = sender;
let sender_user = body.sender_user();
What's the difference here?
the important part is the line below -
(user, device) = sender
panics if there's no device. Splitting it out allows device to be a reference without expecting it to existI see, because
sender()
callssender_device()
which unwraps:Precisely yeah. That's what was causing the issue here (and presumably will cause the same issue elsewhere, however I checked and this isn't referenced anywhere else that appservices are likely to hit)
@ -139,0 +136,4 @@
} else {
lazy_loading::Context {
user_id: sender_user,
device_id: <&DeviceId>::from(""),
Instead of doing an if statement, use a method like unwrap_or_default
@ -8,3 +8,3 @@
use std::sync::OnceLock;
static BRANDING: &str = "conduwuit";
static BRANDING: &str = "continuwuity";
Strictly should be a separate commit, but I don't really care
I can pull that out and push it in later, but it seems a bit silly making a whole new PR for it and might as well shoehorn it in this one
You can have multiple commits in a PR! As said, dw about it but it's not something to make a habit of.
did not mean to do that whoopsies
Undeleted :)
I have been writing software to a professional standard for half a decade and I can be trusted with git, I swear
What's the mystery commit with no content?
Couldn't push my local copy without it for whatever god forsaken reason, do you want me to rewrite history?
nah, I'll do that on merge
Latest commit is just a fancy way of writing:
as demanded by clippy btw.
The as_ref was done implicitly in earlier code, but type inference isn't quite good enough for it to work through the unwrap_or_else.
But anyway down to +- 5 from +23 -18!
e9a1bb7f68
to41a5def0ac
7b1d8d56f6
tob77590e072
b77590e072
to4911fc9951
It's been running on my instances for a couple of days now, no problems.
4911fc9951
todc599db19c