Skip to content

Commit 6ec7466

Browse files
CarlSchwanCarlSchwan
committed
refactor(comment): Port away from deprecated event comment constant
Create new events to replace deprecated CommentsEvent constant and use them when creating CommentsEvents. On the listener side, we can't yet use these events as deck still send the old events. Also fixes some issues reported by psalm level 3 on the comment app. Signed-off-by: Carl Schwan <[email protected]>
1 parent 9e32129 commit 6ec7466

File tree

16 files changed

+213
-110
lines changed

16 files changed

+213
-110
lines changed

‎apps/comments/lib/Activity/Provider.php‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
use OCP\L10N\IFactory;
1919

2020
class Provider implements IProvider {
21-
protected ?IL10N $l = null;
22-
2321
public function __construct(
2422
protected IFactory $languageFactory,
2523
protected IURLGenerator $url,
@@ -42,9 +40,9 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null):
4240
throw new UnknownActivityException();
4341
}
4442

45-
$this->l = $this->languageFactory->get('comments', $language);
46-
4743
if ($event->getSubject() === 'add_comment_subject') {
44+
$l = $this->languageFactory->get('comments', $language);
45+
4846
$this->parseMessage($event);
4947
if ($this->activityManager->getRequirePNG()) {
5048
$event->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/comment.png')));
@@ -54,13 +52,13 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null):
5452

5553
if ($this->activityManager->isFormattingFilteredObject()) {
5654
try {
57-
return $this->parseShortVersion($event);
55+
return $this->parseShortVersion($event, $l);
5856
} catch (UnknownActivityException) {
5957
// Ignore and simply use the long version...
6058
}
6159
}
6260

63-
return $this->parseLongVersion($event);
61+
return $this->parseLongVersion($event, $l);
6462
}
6563
throw new UnknownActivityException();
6664

@@ -69,15 +67,15 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null):
6967
/**
7068
* @throws UnknownActivityException
7169
*/
72-
protected function parseShortVersion(IEvent $event): IEvent {
70+
protected function parseShortVersion(IEvent $event, IL10N $l): IEvent {
7371
$subjectParameters = $this->getSubjectParameters($event);
7472

7573
if ($event->getSubject() === 'add_comment_subject') {
7674
if ($subjectParameters['actor'] === $this->activityManager->getCurrentUserId()) {
77-
$event->setRichSubject($this->l->t('You commented'), []);
75+
$event->setRichSubject($l->t('You commented'), []);
7876
} else {
7977
$author = $this->generateUserParameter($subjectParameters['actor']);
80-
$event->setRichSubject($this->l->t('{author} commented'), [
78+
$event->setRichSubject($l->t('{author} commented'), [
8179
'author' => $author,
8280
]);
8381
}
@@ -91,24 +89,24 @@ protected function parseShortVersion(IEvent $event): IEvent {
9189
/**
9290
* @throws UnknownActivityException
9391
*/
94-
protected function parseLongVersion(IEvent $event): IEvent {
92+
protected function parseLongVersion(IEvent $event, IL10N $l): IEvent {
9593
$subjectParameters = $this->getSubjectParameters($event);
9694

9795
if ($event->getSubject() === 'add_comment_subject') {
9896
if ($subjectParameters['actor'] === $this->activityManager->getCurrentUserId()) {
99-
$event->setParsedSubject($this->l->t('You commented on %1$s', [
97+
$event->setParsedSubject($l->t('You commented on %1$s', [
10098
$subjectParameters['filePath'],
10199
]))
102-
->setRichSubject($this->l->t('You commented on {file}'), [
100+
->setRichSubject($l->t('You commented on {file}'), [
103101
'file' => $this->generateFileParameter($subjectParameters['fileId'], $subjectParameters['filePath']),
104102
]);
105103
} else {
106104
$author = $this->generateUserParameter($subjectParameters['actor']);
107-
$event->setParsedSubject($this->l->t('%1$s commented on %2$s', [
105+
$event->setParsedSubject($l->t('%1$s commented on %2$s', [
108106
$author['name'],
109107
$subjectParameters['filePath'],
110108
]))
111-
->setRichSubject($this->l->t('{author} commented on {file}'), [
109+
->setRichSubject($l->t('{author} commented on {file}'), [
112110
'author' => $author,
113111
'file' => $this->generateFileParameter($subjectParameters['fileId'], $subjectParameters['filePath']),
114112
]);

‎apps/comments/lib/Activity/Setting.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
class Setting extends ActivitySettings {
1313
public function __construct(
14-
protected IL10N $l,
14+
protected readonly IL10N $l,
1515
) {
1616
}
1717

@@ -23,11 +23,11 @@ public function getName(): string {
2323
return $this->l->t('<strong>Comments</strong> for files');
2424
}
2525

26-
public function getGroupIdentifier() {
26+
public function getGroupIdentifier(): string {
2727
return 'files';
2828
}
2929

30-
public function getGroupName() {
30+
public function getGroupName(): string {
3131
return $this->l->t('Files');
3232
}
3333

‎apps/comments/lib/Listener/CommentsEntityEventListener.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ public function handle(Event $event): void {
2727
return;
2828
}
2929

30+
if ($this->userId === null) {
31+
return;
32+
}
33+
3034
$event->addEntityCollection('files', function ($name): bool {
3135
$nodes = $this->rootFolder->getUserFolder($this->userId)->getById((int)$name);
3236
return !empty($nodes);

‎apps/comments/lib/Listener/CommentsEventListener.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ public function handle(Event $event): void {
3535
}
3636

3737
$eventType = $event->getEvent();
38-
if ($eventType === CommentsEvent::EVENT_ADD
39-
) {
38+
if ($eventType === CommentsEvent::EVENT_ADD) {
4039
$this->notificationHandler($event);
4140
$this->activityHandler($event);
4241
return;

‎apps/comments/tests/Unit/Activity/ListenerTest.php‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
use OCP\Activity\IEvent;
1313
use OCP\Activity\IManager;
1414
use OCP\App\IAppManager;
15-
use OCP\Comments\CommentsEvent;
15+
use OCP\Comments\Events\CommentAddedEvent;
1616
use OCP\Comments\IComment;
1717
use OCP\Files\Config\ICachedMountFileInfo;
1818
use OCP\Files\Config\IMountProviderCollection;
@@ -66,14 +66,7 @@ public function testCommentEvent(): void {
6666
->method('getObjectType')
6767
->willReturn('files');
6868

69-
/** @var CommentsEvent|MockObject $event */
70-
$event = $this->createMock(CommentsEvent::class);
71-
$event->expects($this->any())
72-
->method('getComment')
73-
->willReturn($comment);
74-
$event->expects($this->any())
75-
->method('getEvent')
76-
->willReturn(CommentsEvent::EVENT_ADD);
69+
$event = new CommentAddedEvent($comment);
7770

7871
/** @var IUser|MockObject $ownerUser */
7972
$ownerUser = $this->createMock(IUser::class);

‎apps/comments/tests/Unit/EventHandlerTest.php‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
use OCA\Comments\Listener\CommentsEventListener;
1313
use OCA\Comments\Notification\Listener as NotificationListener;
1414
use OCP\Comments\CommentsEvent;
15+
use OCP\Comments\Events\BeforeCommentUpdatedEvent;
16+
use OCP\Comments\Events\CommentAddedEvent;
17+
use OCP\Comments\Events\CommentDeletedEvent;
18+
use OCP\Comments\Events\CommentUpdatedEvent;
1519
use OCP\Comments\IComment;
1620
use PHPUnit\Framework\MockObject\MockObject;
1721
use Test\TestCase;
@@ -50,10 +54,10 @@ public function testNotFiles(): void {
5054

5155
public static function handledProvider(): array {
5256
return [
53-
[CommentsEvent::EVENT_DELETE],
54-
[CommentsEvent::EVENT_UPDATE],
55-
[CommentsEvent::EVENT_PRE_UPDATE],
56-
[CommentsEvent::EVENT_ADD]
57+
['delete'],
58+
['update'],
59+
['pre_update'],
60+
['add']
5761
];
5862
}
5963

@@ -65,14 +69,12 @@ public function testHandled(string $eventType): void {
6569
->method('getObjectType')
6670
->willReturn('files');
6771

68-
/** @var CommentsEvent|MockObject $event */
69-
$event = $this->createMock(CommentsEvent::class);
70-
$event->expects($this->atLeastOnce())
71-
->method('getComment')
72-
->willReturn($comment);
73-
$event->expects($this->atLeastOnce())
74-
->method('getEvent')
75-
->willReturn($eventType);
72+
$event = match ($eventType) {
73+
'add' => new CommentAddedEvent($comment),
74+
'pre_update' => new BeforeCommentUpdatedEvent($comment),
75+
'update' => new CommentUpdatedEvent($comment),
76+
'delete' => new CommentDeletedEvent($comment),
77+
};
7678

7779
$this->notificationListener->expects($this->once())
7880
->method('evaluate')

‎apps/comments/tests/Unit/Notification/ListenerTest.php‎

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@
88
namespace OCA\Comments\Tests\Unit\Notification;
99

1010
use OCA\Comments\Notification\Listener;
11-
use OCP\Comments\CommentsEvent;
11+
use OCP\Comments\Events\BeforeCommentUpdatedEvent;
12+
use OCP\Comments\Events\CommentAddedEvent;
13+
use OCP\Comments\Events\CommentDeletedEvent;
14+
use OCP\Comments\Events\CommentUpdatedEvent;
1215
use OCP\Comments\IComment;
1316
use OCP\IURLGenerator;
1417
use OCP\IUserManager;
@@ -37,10 +40,10 @@ protected function setUp(): void {
3740

3841
public static function eventProvider(): array {
3942
return [
40-
[CommentsEvent::EVENT_ADD, 'notify'],
41-
[CommentsEvent::EVENT_UPDATE, 'notify'],
42-
[CommentsEvent::EVENT_PRE_UPDATE, 'markProcessed'],
43-
[CommentsEvent::EVENT_DELETE, 'markProcessed']
43+
['add', 'notify'],
44+
['update', 'notify'],
45+
['pre_update', 'markProcessed'],
46+
['delete', 'markProcessed']
4447
];
4548
}
4649

@@ -49,7 +52,7 @@ public static function eventProvider(): array {
4952
* @param string $notificationMethod
5053
*/
5154
#[\PHPUnit\Framework\Attributes\DataProvider('eventProvider')]
52-
public function testEvaluate($eventType, $notificationMethod): void {
55+
public function testEvaluate(string $eventType, $notificationMethod): void {
5356
/** @var IComment|MockObject $comment */
5457
$comment = $this->createMock(IComment::class);
5558
$comment->expects($this->any())
@@ -72,14 +75,12 @@ public function testEvaluate($eventType, $notificationMethod): void {
7275
->method('getId')
7376
->willReturn('1234');
7477

75-
/** @var CommentsEvent|MockObject $event */
76-
$event = $this->createMock(CommentsEvent::class);
77-
$event->expects($this->once())
78-
->method('getComment')
79-
->willReturn($comment);
80-
$event->expects(($this->any()))
81-
->method(('getEvent'))
82-
->willReturn($eventType);
78+
$event = match ($eventType) {
79+
'add' => new CommentAddedEvent($comment),
80+
'pre_update' => new BeforeCommentUpdatedEvent($comment),
81+
'update' => new CommentUpdatedEvent($comment),
82+
'delete' => new CommentDeletedEvent($comment),
83+
};
8384

8485
/** @var INotification|MockObject $notification */
8586
$notification = $this->createMock(INotification::class);
@@ -124,14 +125,12 @@ public function testEvaluateNoMentions(string $eventType): void {
124125
->method('getMentions')
125126
->willReturn([]);
126127

127-
/** @var CommentsEvent|MockObject $event */
128-
$event = $this->createMock(CommentsEvent::class);
129-
$event->expects($this->once())
130-
->method('getComment')
131-
->willReturn($comment);
132-
$event->expects(($this->any()))
133-
->method(('getEvent'))
134-
->willReturn($eventType);
128+
$event = match ($eventType) {
129+
'add' => new CommentAddedEvent($comment),
130+
'pre_update' => new BeforeCommentUpdatedEvent($comment),
131+
'update' => new CommentUpdatedEvent($comment),
132+
'delete' => new CommentDeletedEvent($comment),
133+
};
135134

136135
$this->notificationManager->expects($this->never())
137136
->method('createNotification');
@@ -162,14 +161,7 @@ public function testEvaluateUserDoesNotExist(): void {
162161
->method('getId')
163162
->willReturn('1234');
164163

165-
/** @var CommentsEvent|MockObject $event */
166-
$event = $this->createMock(CommentsEvent::class);
167-
$event->expects($this->once())
168-
->method('getComment')
169-
->willReturn($comment);
170-
$event->expects(($this->any()))
171-
->method(('getEvent'))
172-
->willReturn(CommentsEvent::EVENT_ADD);
164+
$event = new CommentAddedEvent($comment);
173165

174166
/** @var INotification|MockObject $notification */
175167
$notification = $this->createMock(INotification::class);

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,10 @@
268268
'OCP\\Command\\ICommand' => $baseDir . '/lib/public/Command/ICommand.php',
269269
'OCP\\Comments\\CommentsEntityEvent' => $baseDir . '/lib/public/Comments/CommentsEntityEvent.php',
270270
'OCP\\Comments\\CommentsEvent' => $baseDir . '/lib/public/Comments/CommentsEvent.php',
271+
'OCP\\Comments\\Events\\BeforeCommentUpdatedEvent' => $baseDir . '/lib/public/Comments/Events/BeforeCommentUpdatedEvent.php',
272+
'OCP\\Comments\\Events\\CommentAddedEvent' => $baseDir . '/lib/public/Comments/Events/CommentAddedEvent.php',
273+
'OCP\\Comments\\Events\\CommentDeletedEvent' => $baseDir . '/lib/public/Comments/Events/CommentDeletedEvent.php',
274+
'OCP\\Comments\\Events\\CommentUpdatedEvent' => $baseDir . '/lib/public/Comments/Events/CommentUpdatedEvent.php',
271275
'OCP\\Comments\\IComment' => $baseDir . '/lib/public/Comments/IComment.php',
272276
'OCP\\Comments\\ICommentsEventHandler' => $baseDir . '/lib/public/Comments/ICommentsEventHandler.php',
273277
'OCP\\Comments\\ICommentsManager' => $baseDir . '/lib/public/Comments/ICommentsManager.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,10 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
309309
'OCP\\Command\\ICommand' => __DIR__ . '/../../..' . '/lib/public/Command/ICommand.php',
310310
'OCP\\Comments\\CommentsEntityEvent' => __DIR__ . '/../../..' . '/lib/public/Comments/CommentsEntityEvent.php',
311311
'OCP\\Comments\\CommentsEvent' => __DIR__ . '/../../..' . '/lib/public/Comments/CommentsEvent.php',
312+
'OCP\\Comments\\Events\\BeforeCommentUpdatedEvent' => __DIR__ . '/../../..' . '/lib/public/Comments/Events/BeforeCommentUpdatedEvent.php',
313+
'OCP\\Comments\\Events\\CommentAddedEvent' => __DIR__ . '/../../..' . '/lib/public/Comments/Events/CommentAddedEvent.php',
314+
'OCP\\Comments\\Events\\CommentDeletedEvent' => __DIR__ . '/../../..' . '/lib/public/Comments/Events/CommentDeletedEvent.php',
315+
'OCP\\Comments\\Events\\CommentUpdatedEvent' => __DIR__ . '/../../..' . '/lib/public/Comments/Events/CommentUpdatedEvent.php',
312316
'OCP\\Comments\\IComment' => __DIR__ . '/../../..' . '/lib/public/Comments/IComment.php',
313317
'OCP\\Comments\\ICommentsEventHandler' => __DIR__ . '/../../..' . '/lib/public/Comments/ICommentsEventHandler.php',
314318
'OCP\\Comments\\ICommentsManager' => __DIR__ . '/../../..' . '/lib/public/Comments/ICommentsManager.php',

‎lib/private/Comments/Manager.php‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99

1010
use OCP\AppFramework\Utility\ITimeFactory;
1111
use OCP\Comments\CommentsEvent;
12+
use OCP\Comments\Events\BeforeCommentUpdatedEvent;
13+
use OCP\Comments\Events\CommentAddedEvent;
14+
use OCP\Comments\Events\CommentDeletedEvent;
15+
use OCP\Comments\Events\CommentUpdatedEvent;
1216
use OCP\Comments\IComment;
1317
use OCP\Comments\ICommentsEventHandler;
1418
use OCP\Comments\ICommentsManager;
@@ -902,7 +906,7 @@ public function delete($id) {
902906
if ($comment->getVerb() === 'reaction_deleted') {
903907
$this->deleteReaction($comment);
904908
}
905-
$this->sendEvent(CommentsEvent::EVENT_DELETE, $comment);
909+
$this->sendEvent(new CommentDeletedEvent($comment));
906910
}
907911

908912
return ($affectedRows > 0);
@@ -1147,7 +1151,7 @@ protected function insert(IComment $comment): bool {
11471151
if ($comment->getVerb() === 'reaction') {
11481152
$this->addReaction($comment);
11491153
}
1150-
$this->sendEvent(CommentsEvent::EVENT_ADD, $comment);
1154+
$this->sendEvent(new CommentAddedEvent($comment));
11511155
}
11521156

11531157
return $affectedRows > 0;
@@ -1233,11 +1237,11 @@ private function sumReactions(string $parentId): void {
12331237
* @return bool
12341238
* @throws NotFoundException
12351239
*/
1236-
protected function update(IComment $comment) {
1240+
protected function update(IComment $comment): bool {
12371241
// for properly working preUpdate Events we need the old comments as is
12381242
// in the DB and overcome caching. Also avoid that outdated information stays.
12391243
$this->uncache($comment->getId());
1240-
$this->sendEvent(CommentsEvent::EVENT_PRE_UPDATE, $this->get($comment->getId()));
1244+
$this->sendEvent(new BeforeCommentUpdatedEvent($this->get($comment->getId())));
12411245
$this->uncache($comment->getId());
12421246

12431247
$result = $this->updateQuery($comment);
@@ -1246,7 +1250,7 @@ protected function update(IComment $comment) {
12461250
$this->deleteReaction($comment);
12471251
}
12481252

1249-
$this->sendEvent(CommentsEvent::EVENT_UPDATE, $comment);
1253+
$this->sendEvent(new CommentUpdatedEvent($comment));
12501254

12511255
return $result;
12521256
}
@@ -1528,14 +1532,10 @@ private function getEventHandlers() {
15281532
}
15291533

15301534
/**
1531-
* sends notifications to the registered entities
1532-
*
1533-
* @param $eventType
1534-
* @param IComment $comment
1535+
* Sends notifications to the registered entities
15351536
*/
1536-
private function sendEvent($eventType, IComment $comment) {
1537+
private function sendEvent(CommentsEvent $event): void {
15371538
$entities = $this->getEventHandlers();
1538-
$event = new CommentsEvent($eventType, $comment);
15391539
foreach ($entities as $entity) {
15401540
$entity->handle($event);
15411541
}

0 commit comments

Comments
 (0)