TODO: atm this still refreshes all users actually actively
sending stuff to the inbox, but this will change with the
signing key migration (only user/inbox endpoints still do)
In theory a pedantic reading of the spec indeed suggests
DMs must only be delivered to personal inboxes. However,
in practice the normative force of real-world implementations
disagrees. Mastodon, Iceshrimp.NET and GtS (the latter notably has a
config option to never use sharedInboxes) all unconditionally prefer
sharedInbox for everything without ill effect. This saves on duplicate
deliveries on the sending and processing on the receiving end.
(Typically the receiving side ends up rejecting
all but the first copy as duplicates)
Furthermore current determine_inbox logic also actually needs up
forcing personal inboxes for follower-only posts, unless they
additionally explicitly address at least one specific actor.
This is even much wasteful and directly contradicts
the explicit intent of the spec.
There’s one part where the use of sharedInbox falls apart,
namely spec-compliant bcc and bto addressing. AP spec requires
bcc/bto fields to be stripped before delivery and then implicitly
reconstructed by the receiver based on the addressed personal inbox.
In practice however, this addressing mode is almost unused. Neither of
the three implementations brought up above supports it and while *oma
does use bcc for list addressing, it does not use it in a spec-compliant
way and even copies same-host recipients into cc before delivery.
Messages with bcc addressing are handled in another function clause,
always force personal inboxes for every recipient and not affected by
this commit.
In theory it would be beneficial to use sharedInbox there too for all
but bcc recipients. But in practice list addressing has been broken for
quite some time already and is not actually exposed in any frontend,
as discussed in #812.
Therefore any changes here have virtually no effect anyway
and all code concerning it may just be outright removed.
Otherwise attachments have a high chance to disappear with akkoma-fe’s
“delete & redraft” feature when cleanup is enabled in the backend. Since
we don't know whether a deletion was intended to be part of a redraft
process or even if whether the redraft was abandoned we still have to
delete attachments eventually.
A thirty minute delay should provide sufficient time for redrafting.
Fixes: #775
The cleanup attachment worker was run for every deleted post,
even if it’s a remote post whose attachments we don't even store.
This was especially bad due to attachment cleanup involving a
particularly heavy query wasting a bunch of database perf for nil.
This was uncovered by comparing statistics from
#784 and
#765 (comment)
Currently logic unconditionally adds public adressing to "cc"
and follower adressing to "to" after attempting to strip it
from the other one. This is horrible for two reasons:
First the bug prompting this investigation and fix,
this creates duplicates when adressing URIs already
were in their intended final field; e.g. prominently
the case for all "unlisted" posts.
Since List.delete only removes the first occurence,
this then broke follower-adress stripping later on.
It’s also not safe in general wrt to non-public adressing:
e.g. pre-existing duplicates didn’t get fully stripped,
bespoke adressing modes with only one of public addressing
or follower addressing — and most importantly:
any belatedly received DM also got public adressing added!
Shockingly this last point was actually checked as "correct" in tests
albeit this appears to be a mistake from mindless match adjustments
from when genuine crashes on nil adressing lists got fixed in
10c792110e.
Clean up this sloppy logic up, making sure all instances of relevant
adresses are purged and only readded when they actually existed to begin
with. To avoid problems with any List.delete usages remaining in other
places, also ensure we never create duplicate entries.
Turns out this is also used to set the default values in adminfe, so
update this to the correct URL for the NSFW banner so that it doesn't
break if you reset this value.
Pruning can go on for a long time; give admins some insight into that
something is happening to make it less frustrating and to make it easier
which part of the process is stalled should this happen.
Again most of the changes are merely reindents;
review with whitespace changes hidden recommended.
May sometimes be helpful to get more predictable runtime
than just with an age-based limit.
The subquery for the non-keep-threads path is required
since delte_all does not directly accept limit().
Again most of the diff is just adjusting indentation, best
hide whitespace-only changes with git diff -w or similar.
This gives feedback when to stop rerunning limited batches.
Most of the diff is just adjusting indentation; best reviewed
with whitespace-only changes hidden, e.g. `git diff -w`.
This part of pruning can be very expensive and bog down the whole
instance to an unusable sate for a long time. It can thus be desireable
to split it from prune_objects and run it on its own in smaller limited batches.
If the batches are smaller enough and spaced out a bit, it may even be possible
to avoid any downtime. If not, the limit can still help to at least make the
downtime duration somewhat more predictable.
May sometimes be helpful to get more predictable runtime
than just with an age-based limit.
The subquery for the non-keep-threads path is required
since delte_all does not directly accept limit().
Again most of the diff is just adjusting indentation, best
hide whitespace-only changes with git diff -w or similar.
This gives feedback when to stop rerunning limited batches.
Most of the diff is just adjusting indentation; best reviewed
with whitespace-only changes hidden, e.g. `git diff -w`.
This part of pruning can be very expensive and bog down the whole
instance to an unusable sate for a long time. It can thus be desireable
to split it from prune_objects and run it on its own in smaller limited batches.
If the batches are smaller enough and spaced out a bit, it may even be possible
to avoid any downtime. If not, the limit can still help to at least make the
downtime duration somewhat more predictable.
