GNUsocial/gnu-social
4
0
Fork 1
Code Issues Releases Wiki Activity

[COMPONENT][Notification] Make logic more generic and robust

Browse Source 
Fixed various bugs

Some important concepts to bear in mind:

* Notification: Associated with activities, won't be reconstructed
together with objects, can be thought of as transient

* Attention: Associated with objects, will be reconstructed with them, can
be thought as persistent

* Notifications and Attentions have no direct implications.

* Mentions are a specific form of attentions in notes, leads to the creation of Attentions.

Finally,

Potential PHP issue detected and reported: https://github.com/php/php-src/issues/8199
`static::method()` from a non static context (such as a class method) calls `__call`, rather than
the expected `__callStatic`. Can be fixed by using `(static fn() => static::method())()`, but the
usage of the magic method is strictly unnecessary in this case.
This commit is contained in:
Diogo Peralta Cordeiro
2022-03-13 18:23:19 +00:00
parent e1cceac150
commit 888c3798b7
32 changed files with 438 additions and 494 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
components/Blog/Controller/Post.php
View File

@@ -145,7 +145,7 @@ class Post extends Controller
content_type: $content_type,
locale: $data['language'],
scope: VisibilityScope::from($data['visibility']),
targets: [(int) $data['in']],
attentions: [(int) $data['in']],
reply_to: $data['reply_to_id'],
attachments: $data['attachments'],
process_note_content_extra_args: $extra_args,

9
components/Group/Group.php
View File

@@ -51,12 +51,17 @@ class Group extends Component
* Enqueues a notification for an Actor (such as person or group) which means
* it shows up in their home feed and such.
*/
public function onNewNotificationWithTargets(Actor $sender, Activity $activity, array $targets = [], ?string $reason = null): bool
public function onNewNotificationStart(Actor $sender, Activity $activity, array $targets = [], ?string $reason = null): bool
{
foreach ($targets as $target) {
if ($target->isGroup()) {
// The Group announces to its subscribers
Notification::notify($target, $activity, $target->getSubscribers(), $reason);
Notification::notify(
sender: $target,
activity: $activity,
targets: $target->getSubscribers(),
reason: $reason,
);
}
}

4
components/Notification/Controller/Feed.php
View File

@@ -53,9 +53,9 @@ class Feed extends Controller
WHERE n.id IN (
SELECT act.object_id FROM \App\Entity\Activity AS act
WHERE act.object_type = 'note' AND act.id IN
(SELECT att.activity_id FROM \Component\Notification\Entity\Notification AS att WHERE att.target_id = :id)
(SELECT att.activity_id FROM \Component\Notification\Entity\Notification AS att WHERE att.target_id = :target_id)
)
EOF, ['id' => $user->getId()]);
EOF, [':target' => $user->getId()]);
return [
'_template' => 'collection/notes.html.twig',
'page_title' => _m('Notifications'),

47
components/Notification/Entity/Attention.php
View File

@@ -24,11 +24,21 @@ namespace Component\Notification\Entity;
use App\Core\Entity;
/**
* Entity for note attentions
* Entity for object attentions
*
* An attention is a form of persistent notification.
* It exists together and for as long as the object it belongs to.
* Creating an attention requires creating a Notification.
*
* @category DB
* @package GNUsocial
*
* @author Zach Copley <zach@status.net>
* @copyright 2010 StatusNet Inc.
* @author Mikael Nordfeldth <mmn@hethane.se>
* @copyright 2009-2014 Free Software Foundation, Inc http://www.fsf.org
* @author Diogo Peralta Cordeiro <@diogo.site>
* @copyright 2021-2022 Free Software Foundation, Inc http://www.fsf.org
* @author Diogo Peralta Cordeiro <@diogo.site>
* @copyright 2022 Free Software Foundation, Inc http://www.fsf.org
* @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
@@ -37,18 +47,30 @@ class Attention extends Entity
{
// {{{ Autocode
// @codeCoverageIgnoreStart
private int $note_id;
private string $object_type;
private int $object_id;
private int $target_id;
public function setNoteId(int $note_id): self
public function setObjectType(string $object_type): self
{
$this->note_id = $note_id;
$this->object_type = mb_substr($object_type, 0, 32);
return $this;
}
public function getNoteId(): int
public function getObjectType(): string
{
return $this->note_id;
return $this->object_type;
}
public function setObjectId(int $object_id): self
{
$this->object_id = $object_id;
return $this;
}
public function getObjectId(): int
{
return $this->object_id;
}
public function setTargetId(int $target_id): self
@@ -68,15 +90,16 @@ class Attention extends Entity
public static function schemaDef(): array
{
return [
'name' => 'note_attention',
'description' => 'Note attentions to actors (that are not a mention)',
'name' => 'attention',
'description' => 'Attentions to actors (these are not mentions)',
'fields' => [
'note_id' => ['type' => 'int', 'foreign key' => true, 'target' => 'Note.id', 'multiplicity' => 'one to one', 'not null' => true, 'description' => 'note_id to give attention'],
'target_id' => ['type' => 'int', 'foreign key' => true, 'target' => 'Actor.id', 'multiplicity' => 'one to one', 'not null' => true, 'description' => 'actor_id for feed receiver'],
'object_type' => ['type' => 'varchar', 'length' => 32, 'not null' => true, 'description' => 'the name of the table this object refers to'],
'object_id' => ['type' => 'int', 'not null' => true, 'description' => 'id in the referenced table'],
'target_id' => ['type' => 'int', 'foreign key' => true, 'target' => 'Actor.id', 'multiplicity' => 'one to one', 'not null' => true, 'description' => 'actor_id for feed receiver'],
],
'primary key' => ['note_id', 'target_id'],
'primary key' => ['object_type', 'object_id', 'target_id'],
'indexes' => [
'attention_note_id_idx' => ['note_id'],
'attention_object_id_idx' => ['object_id'],
'attention_target_id_idx' => ['target_id'],
],
];

14
components/Notification/Entity/Notification.php
View File

@@ -28,17 +28,17 @@ use App\Entity\Actor;
use DateTimeInterface;
/**
* Entity for attentions
* Entity for Notifications
*
* A Notification when isolated is a form of transient notification.
* When together with a persistent form of notification such as attentions or mentions,
* it records that the target was notified - which avoids re-notifying upon objects reconstructions.
*
* @category DB
* @package GNUsocial
*
* @author Zach Copley <zach@status.net>
* @copyright 2010 StatusNet Inc.
* @author Mikael Nordfeldth <mmn@hethane.se>
* @copyright 2009-2014 Free Software Foundation, Inc http://www.fsf.org
* @author Hugo Sales <hugo@hsal.es>
* @copyright 2020-2021 Free Software Foundation, Inc http://www.fsf.org
* @author Diogo Peralta Cordeiro <@diogo.site>
* @copyright 2021-2022 Free Software Foundation, Inc http://www.fsf.org
* @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
*/
class Notification extends Entity

52
components/Notification/Notification.php
View File

@@ -64,14 +64,43 @@ class Notification extends Component
/**
* Enqueues a notification for an Actor (such as person or group) which means
* it shows up in their home feed and such.
* WARNING: It's highly advisable to have flushed any relevant objects before triggering this event.
* OBSERVATION: $sender->getSubscribers() will always be pre-included, thus why $targets=[] is normal
*
* $targets should be of the shape:
* ['source' => (int|Actor)[]] // Prefer Actor whenever possible
* Example of $targets:
* [[42, $actor_alice, $actor_bob]] // Avoid repeating actors or ids
*
* @param Actor $sender The one responsible for this activity, take care not to include it in targets
* @param Activity $activity The activity responsible for the object being given to known to targets
* @param array $targets attentions, Mentions, any other source
* @param null|string $reason An optional reason explaining why this notification exists
*/
public function onNewNotification(Actor $sender, Activity $activity, array $ids_already_known = [], ?string $reason = null): bool
public function onNewNotification(Actor $sender, Activity $activity, array $targets = [], ?string $reason = null): bool
{
$targets = $activity->getNotificationTargets(ids_already_known: $ids_already_known, sender_id: $sender->getId());
if (Event::handle('NewNotificationWithTargets', [$sender, $activity, $targets, $reason]) === Event::next) {
self::notify($sender, $activity, $targets, $reason);
// Ensure targets are all actor objects and unique
$effective_targets = [];
foreach ($targets as $target) {
if (\is_int($target)) {
$target_id = $target;
$target_object = null;
} else {
$target_id = $target->getId();
$target_object = $target;
}
if (!\array_key_exists(key: $target_id, array: $effective_targets)) {
$target_object ??= Actor::getById($target_id);
$effective_targets[$target_id] = $target_object;
}
}
unset($targets);
if (Event::handle('NewNotificationStart', [$sender, $activity, $effective_targets, $reason]) === Event::next) {
self::notify($sender, $activity, $effective_targets, $reason);
}
Event::handle('NewNotificationEnd', [$sender, $activity, $effective_targets, $reason]);
return Event::next;
}
@@ -91,7 +120,8 @@ class Notification extends Component
}
/**
* Bring given Activity to Targets's attention
* Bring given Activity to Targets' knowledge.
* This will flush a Notification to DB.
*
* @return true if successful, false otherwise
*/
@@ -100,8 +130,8 @@ class Notification extends Component
$remote_targets = [];
foreach ($targets as $target) {
if ($target->getIsLocal()) {
if ($target->hasBlocked($activity->getActor())) {
Log::info("Not saving reply to actor {$target->getId()} from sender {$sender->getId()} because of a block.");
if ($target->hasBlocked($author = $activity->getActor())) {
Log::info("Not saving notification to actor {$target->getId()} from sender {$sender->getId()} because receiver blocked author {$author->getId()}.");
continue;
}
if (Event::handle('NewNotificationShould', [$activity, $target]) === Event::next) {
@@ -113,7 +143,7 @@ class Notification extends Component
}
Queue::enqueue(
payload: [$sender, $activity, $target, $reason],
queue: 'notification_local',
queue: 'NotificationLocal',
priority: true,
);
} else {
@@ -124,7 +154,7 @@ class Notification extends Component
}
// XXX: Unideal as in failures the rollback will leave behind a false notification,
// but most notifications (all) require flushing the objects first
// Should be okay as long as implementors bear this in mind
// Should be okay as long as implementations bear this in mind
try {
DB::wrapInTransaction(fn () => DB::persist(Entity\Notification::create([
'activity_id' => $activity->getId(),
@@ -132,7 +162,7 @@ class Notification extends Component
'reason' => $reason,
])));
} catch (Exception|Throwable $e) {
// We do our best not to record duplicated notifications, but it's not insane that can happen
// We do our best not to record duplicate notifications, but it's not insane that can happen
Log::error('It was attempted to record an invalid notification!', [$e]);
}
}
@@ -140,7 +170,7 @@ class Notification extends Component
if ($remote_targets !== []) {
Queue::enqueue(
payload: [$sender, $activity, $remote_targets, $reason],
queue: 'notification_remote',
queue: 'NotificationRemote',
priority: false,
);
}

6
components/Posting/Controller/Posting.php
View File

@@ -52,7 +52,7 @@ class Posting extends Controller
content_type: $data['content_type'],
locale: $data['language'],
scope: VisibilityScope::from($data['visibility']),
targets: isset($target) ? [$target] : [],
attentions: isset($target) ? [$target] : [],
reply_to: \array_key_exists('reply_to_id', $data) ? $data['reply_to_id'] : null,
attachments: $data['attachments'],
process_note_content_extra_args: $extra_args,
@@ -61,9 +61,9 @@ class Posting extends Controller
return Core\Form::forceRedirect($form, $request);
}
} catch (FormSizeFileException $e) {
throw new ClientException(_m('Invalid file size given'), previous: $e);
throw new ClientException(_m('Invalid file size given.'), previous: $e);
}
}
throw new ClientException(_m('Invalid form submission'));
throw new ClientException(_m('Invalid form submission.'));
}
}

79
components/Posting/Posting.php
View File

@@ -48,7 +48,6 @@ use Component\Attachment\Entity\AttachmentToNote;
use Component\Conversation\Conversation;
use Component\Language\Entity\Language;
use Component\Notification\Entity\Attention;
use Functional as F;
use Symfony\Component\HttpFoundation\File\UploadedFile;
use Symfony\Component\HttpFoundation\Request;
@@ -94,7 +93,7 @@ class Posting extends Component
string $content_type,
?string $locale = null,
?VisibilityScope $scope = null,
array $targets = [],
array $attentions = [],
null|int|Note $reply_to = null,
array $attachments = [],
array $processed_attachments = [],
@@ -104,13 +103,13 @@ class Posting extends Component
string $source = 'web',
?string $title = null,
): array {
[$activity, $note, $attention_ids] = self::storeLocalNote(
[$activity, $note, $effective_attentions] = self::storeLocalNote(
actor: $actor,
content: $content,
content_type: $content_type,
locale: $locale,
scope: $scope,
targets: $targets,
attentions: $attentions,
reply_to: $reply_to,
attachments: $attachments,
processed_attachments: $processed_attachments,
@@ -125,10 +124,18 @@ class Posting extends Component
if ($flush_and_notify) {
// Flush before notification
DB::flush();
Event::handle('NewNotification', [$actor, $activity, ['object' => $attention_ids], _m('{nickname} created a page {note_id}.', ['{nickname}' => $actor->getNickname(), '{note_id}' => $activity->getObjectId()])]);
Event::handle('NewNotification', [
$actor,
$activity,
$effective_attentions,
_m('Actor {actor_id} created page {note_id}.', [
'{actor_id}' => $actor->getId(),
'{note_id}' => $activity->getObjectId(),
]),
]);
}
return [$activity, $note, $attention_ids];
return [$activity, $note, $effective_attentions];
}
/**
@@ -141,7 +148,7 @@ class Posting extends Component
* @param string $content_type Indicating one of the various supported content format (Plain Text, Markdown, LaTeX...)
* @param null|string $locale Note's written text language, set by the default Actor language or upon filling
* @param null|VisibilityScope $scope The visibility of this Note
* @param array $targets Actor|int[]: In Group/To Person or Bot, registers an attention between note and target
* @param array $attentions Actor|int[]: In Group/To Person or Bot, registers an attention between note and target
* @param null|int|Note $reply_to The soon-to-be Note parent's id, if it's a Reply itself
* @param array $attachments UploadedFile[] to be stored as GSFiles associated to this note
* @param array $processed_attachments Array of [Attachment, Attachment's name][] to be associated to this $actor and Note
@@ -154,7 +161,7 @@ class Posting extends Component
* @throws DuplicateFoundException
* @throws ServerException
*
* @return array [Activity, Note, int[]] Activity, Note, Attention Ids
* @return array [Activity, Note, Effective Attentions]
*/
public static function storeLocalNote(
Actor $actor,
@@ -162,7 +169,7 @@ class Posting extends Component
string $content_type,
?string $locale = null,
?VisibilityScope $scope = null,
array $targets = [],
array $attentions = [],
null|int|Note $reply_to = null,
array $attachments = [],
array $processed_attachments = [],
@@ -209,7 +216,7 @@ class Posting extends Component
if (!\is_null($reply_to_id)) {
Cache::incr(Note::cacheKeys($reply_to_id)['replies-count']);
// Not having them cached doesn't mean replies don't exist, but don't push it to the
// list, as that means they need to be refetched, or some would be missed
// list, as that means they need to be re-fetched, or some would be missed
if (Cache::exists(Note::cacheKeys($reply_to_id)['replies'])) {
Cache::listPushRight(Note::cacheKeys($reply_to_id)['replies'], $note);
}
@@ -221,12 +228,12 @@ class Posting extends Component
Event::handle('ProcessNoteContent', [$note, $content, $content_type, $process_note_content_extra_args]);
}
// These are note attachments now, and not just attachments, ensure these relations are ensured
// These are note attachments now, and not just attachments, ensure these relations are respected
if ($processed_attachments !== []) {
foreach ($processed_attachments as [$a, $fname]) {
// Most attachments should already be associated with its author, but maybe it didn't make sense
//for this attachment, or it's simply a repost of an attachment by a different actor
if (DB::count('actor_to_attachment', $args = ['attachment_id' => $a->getId(), 'actor_id' => $actor->getId()]) === 0) {
if (DB::count(ActorToAttachment::class, $args = ['attachment_id' => $a->getId(), 'actor_id' => $actor->getId()]) === 0) {
DB::persist(ActorToAttachment::create($args));
}
DB::persist(AttachmentToNote::create(['attachment_id' => $a->getId(), 'note_id' => $note->getId(), 'title' => $fname]));
@@ -242,13 +249,38 @@ class Posting extends Component
]);
DB::persist($activity);
$attention_ids = [];
foreach ($targets as $target) {
$target_id = \is_int($target) ? $target : $target->getId();
DB::persist(Attention::create(['note_id' => $note->getId(), 'target_id' => $target_id]));
$attention_ids[$target_id] = true;
$effective_attentions = [];
foreach ($attentions as $target) {
if (\is_int($target)) {
$target_id = $target;
$add = !\array_key_exists($target_id, $effective_attentions);
$effective_attentions[$target_id] = $target;
} else {
$target_id = $target->getId();
if ($add = !\array_key_exists($target_id, $effective_attentions)) {
$effective_attentions[$target_id] = $target_id;
}
}
if ($add) {
DB::persist(Attention::create(['object_type' => Note::schemaName(), 'object_id' => $note->getId(), 'target_id' => $target_id]));
}
}
foreach ($mentions as $m) {
foreach ($m['mentioned'] ?? [] as $mentioned) {
$target_id = $mentioned->getId();
if (!\array_key_exists($target_id, $effective_attentions)) {
DB::persist(Attention::create(['object_type' => Note::schemaName(), 'object_id' => $note->getId(), 'target_id' => $target_id]));
}
$effective_attentions[$target_id] = $mentioned;
}
}
foreach ($actor->getSubscribers() as $subscriber) {
$target_id = $subscriber->getId();
DB::persist(Attention::create(['object_type' => Activity::schemaName(), 'object_id' => $activity->getId(), 'target_id' => $target_id]));
$effective_attentions[$target_id] = $subscriber;
}
$attention_ids = array_keys($attention_ids);
if ($flush_and_notify) {
// Flush before notification
@@ -256,18 +288,15 @@ class Posting extends Component
Event::handle('NewNotification', [
$actor,
$activity,
[
'note-attention' => $attention_ids,
'object' => F\unique(F\flat_map($mentions, fn (array $m) => F\map($m['mentioned'] ?? [], fn (Actor $a) => $a->getId()))),
],
_m('{nickname} created a note {note_id}.', [
'{nickname}' => $actor->getNickname(),
$effective_attentions,
_m('Actor {actor_id} created note {note_id}.', [
'{actor_id}' => $actor->getId(),
'{note_id}' => $activity->getObjectId(),
]),
]);
}
return [$activity, $note, $attention_ids];
return [$activity, $note, $effective_attentions];
}
public function onRenderNoteContent(string $content, string $content_type, ?string &$rendered, Actor $author, ?string $language = null, array &$mentions = [])

21
components/Subscription/Entity/ActorSubscription.php
View File

@@ -114,27 +114,6 @@ class ActorSubscription extends Entity
];
}
/**
* @see Entity->getNotificationTargetIds
*/
public function getNotificationTargetIds(array $ids_already_known = [], ?int $sender_id = null, bool $include_additional = true): array
{
if (!\array_key_exists('object', $ids_already_known)) {
$target_ids = [$this->getSubscribedId()]; // The object of any subscription is the one subscribed (or unsubscribed)
} else {
$target_ids = $ids_already_known['object'];
}
// Additional actors that should know about this
if ($include_additional && \array_key_exists('additional', $ids_already_known)) {
array_push($target_ids, ...$ids_already_known['additional']);
} else {
return $target_ids;
}
return array_unique($target_ids);
}
public static function schemaDef(): array
{
return [

15
components/Subscription/Subscription.php
View File

@@ -37,6 +37,7 @@ use App\Util\Common;
use App\Util\Exception\DuplicateFoundException;
use App\Util\Exception\NotFoundException;
use App\Util\Exception\ServerException;
use Component\Notification\Entity\Attention;
use Component\Subscription\Controller\Subscribers as SubscribersController;
use Component\Subscription\Controller\Subscriptions as SubscriptionsController;
@@ -97,20 +98,21 @@ class Subscription extends Component
$subscription = DB::findOneBy(table: Entity\ActorSubscription::class, criteria: $opts, return_null: true);
$activity = null;
if (\is_null($subscription)) {
DB::persist(Entity\ActorSubscription::create($opts));
DB::persist($subscription = Entity\ActorSubscription::create($opts));
$activity = Activity::create([
'actor_id' => $subscriber_id,
'verb' => 'subscribe',
'object_type' => 'actor',
'object_type' => Actor::schemaName(),
'object_id' => $subscribed_id,
'source' => $source,
]);
DB::persist($activity);
DB::persist(Attention::create(['object_type' => Activity::schemaName(), 'object_id' => $activity->getId(), 'target_id' => $subscribed_id]));
Event::handle('NewNotification', [
\is_int($subject) ? $subject : Actor::getById($subscriber_id),
$activity,
['object' => [$activity->getObjectId()]],
[$subscribed_id],
_m('{subject} subscribed to {object}.', ['{subject}' => $activity->getActorId(), '{object}' => $activity->getObjectId()]),
]);
}
@@ -146,21 +148,22 @@ class Subscription extends Component
if (!\is_null($subscription)) {
// Remove Subscription
DB::remove($subscription);
$previous_follow_activity = DB::findBy('activity', ['verb' => 'subscribe', 'object_type' => 'actor', 'object_id' => $subscribed_id], order_by: ['created' => 'DESC'])[0];
$previous_follow_activity = DB::findBy(Activity::class, ['verb' => 'subscribe', 'object_type' => Actor::schemaName(), 'object_id' => $subscribed_id], order_by: ['created' => 'DESC'])[0];
// Store Activity
$activity = Activity::create([
'actor_id' => $subscriber_id,
'verb' => 'undo',
'object_type' => 'activity',
'object_type' => Activity::schemaName(),
'object_id' => $previous_follow_activity->getId(),
'source' => $source,
]);
DB::persist($activity);
DB::persist(Attention::create(['object_type' => Activity::schemaName(), 'object_id' => $activity->getId(), 'target_id' => $subscribed_id]));
Event::handle('NewNotification', [
\is_int($subject) ? $subject : Actor::getById($subscriber_id),
$activity,
['object' => [$previous_follow_activity->getObjectId()]],
[],
_m('{subject} unsubscribed from {object}.', ['{subject}' => $activity->getActorId(), '{object}' => $previous_follow_activity->getObjectId()]),
]);
}