Skip to content

Commit 846069a

Browse files
miaulalalamiaulalala
authored
Merge pull request #2418 from nextcloud/backport/2237/stable32
[stable32] fix(bulkactivity): bulk query user settings
2 parents ec2b5cd + f53d262 commit 846069a

2 files changed

Lines changed: 133 additions & 62 deletions

File tree

‎lib/Consumer.php‎

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
use OCP\Activity\IEvent;
1414
use OCP\Activity\IManager;
1515
use OCP\Activity\ISetting;
16-
use Throwable;
16+
use OCP\Config\IUserConfig;
17+
use OCP\Config\ValueType;
18+
use OCP\DB\Exception;
1719

1820
class Consumer implements IConsumer, IBulkConsumer {
1921

@@ -22,6 +24,7 @@ public function __construct(
2224
protected IManager $manager,
2325
protected UserSettings $userSettings,
2426
protected NotificationGenerator $notificationGenerator,
27+
protected IUserConfig $userConfig,
2528
) {
2629
}
2730

@@ -53,56 +56,63 @@ public function receive(IEvent $event): void {
5356
}
5457

5558
/**
56-
* Send an event to the notifications of a user
59+
* Send an event to the notifications of a bulk of users
5760
*
5861
* @param IEvent $event
59-
* @throws Throwable
60-
*
62+
* @param array $affectedUserIds
63+
* @param ISetting $setting
6164
* @return void
65+
* @throws Exception
6266
*/
6367
#[\Override]
6468
public function bulkReceive(IEvent $event, array $affectedUserIds, ISetting $setting): void {
6569
if (empty($affectedUserIds)) {
6670
return;
6771
}
72+
6873
$activityIds = $this->data->bulkSend($event, $affectedUserIds);
6974

7075
if (empty($activityIds)) {
7176
return;
7277
}
7378

7479
$canChangeMail = $setting->canChangeMail();
75-
$canChangePush = false;
76-
if ($setting instanceof ActivitySettings && $setting->canChangeNotification() === true) {
77-
$canChangePush = true;
80+
$canChangePush = $setting instanceof ActivitySettings && $setting->canChangeNotification() === true;
81+
82+
$userPushSettings = $userEmailSettings = $batchTimeSettings = null;
83+
if ($canChangePush === true) {
84+
$userPushSettings = $this->userConfig->getValuesByUsers('activity', 'notify_notification_' . $event->getType(), ValueType::BOOL, $affectedUserIds);
7885
}
7986

80-
if ($canChangePush === false && $canChangeMail === false) {
81-
return;
87+
if ($canChangeMail === true || $setting->isDefaultEnabledMail() === true) {
88+
$userEmailSettings = $this->userConfig->getValuesByUsers('activity', 'notify_email_' . $event->getType(), ValueType::BOOL, $affectedUserIds);
89+
$batchTimeSettings = $this->userConfig->getValuesByUsers('activity', 'notify_setting_batchtime', ValueType::INT, $affectedUserIds);
8290
}
8391

92+
$shouldFlush = $this->notificationGenerator->deferNotifications();
8493
foreach ($activityIds as $activityId => $affectedUser) {
8594
if ($event->getAuthor() === $affectedUser) {
8695
continue;
8796
}
8897
$event->setAffectedUser($affectedUser);
89-
if ($canChangePush === true) {
90-
$notificationSetting = $this->userSettings->getUserSetting($affectedUser, 'notification', $event->getType());
91-
}
92-
93-
if ($canChangeMail === true) {
94-
$emailSetting = $this->userSettings->getUserSetting($event->getAffectedUser(), 'email', $event->getType());
95-
$emailSetting = ($emailSetting) ? $this->userSettings->getUserSetting($event->getAffectedUser(), 'setting', 'batchtime') : false;
98+
$notificationSetting = $userPushSettings[$affectedUser];
99+
if ($notificationSetting !== null) {
100+
$notificationSetting = (bool)$notificationSetting;
96101
}
102+
$emailSetting = $userEmailSettings[$affectedUser] ?? false;
103+
$emailSetting = ($emailSetting) ? ($batchTimeSettings[$affectedUser] ?? false) : false;
97104

98-
if (isset($notificationSetting) && $notificationSetting === true) {
105+
if ($notificationSetting !== false) {
99106
$this->notificationGenerator->sendNotificationForEvent($event, $activityId, $notificationSetting);
100107
}
101108

102109
if (isset($emailSetting) && $emailSetting !== false) {
103-
$latestSend = $event->getTimestamp() + $emailSetting;
110+
$latestSend = (int)($event->getTimestamp() + $emailSetting);
104111
$this->data->storeMail($event, $latestSend);
105112
}
106113
}
114+
if ($shouldFlush === true) {
115+
$this->notificationGenerator->flushNotifications();
116+
}
107117
}
108118
}

‎tests/ConsumerTest.php‎

Lines changed: 105 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,16 @@
2828
use OCA\Activity\Data;
2929
use OCA\Activity\NotificationGenerator;
3030
use OCA\Activity\UserSettings;
31+
use OCP\Activity\ActivitySettings;
32+
use OCP\Activity\IEvent;
3133
use OCP\Activity\IManager;
34+
use OCP\Config\IUserConfig;
35+
use OCP\IDBConnection;
3236
use OCP\IL10N;
3337
use OCP\L10N\IFactory;
38+
use OCP\Server;
39+
use PHPUnit\Framework\Attributes\DataProvider;
40+
use PHPUnit\Framework\Attributes\Group;
3441
use PHPUnit\Framework\MockObject\MockObject;
3542

3643
/**
@@ -40,48 +47,37 @@
4047
* @package OCA\Activity\Tests
4148
*/
4249
class ConsumerTest extends TestCase {
43-
/** @var Consumer */
44-
protected $consumer;
45-
46-
/** @var Data|MockObject */
47-
protected $data;
48-
49-
/** @var IFactory|MockObject */
50-
protected $l10nFactory;
51-
/** @var IManager|MockObject */
52-
protected $activityManager;
53-
54-
/** @var \OCA\Activity\UserSettings */
55-
protected $userSettings;
56-
57-
/** @var NotificationGenerator|MockObject */
58-
protected $notificationGenerator;
50+
protected Data&MockObject $data;
51+
protected MockObject&IFactory $l10nFactory;
52+
protected IManager&MockObject $activityManager;
53+
protected NotificationGenerator&MockObject $notificationGenerator;
54+
protected UserSettings $userSettings;
55+
private IUserConfig&MockObject $userConfig;
56+
private IEvent $event;
57+
private Consumer $consumer;
5958

6059
protected function setUp(): void {
6160
parent::setUp();
6261
$this->deleteTestActivities();
6362

6463
$this->activityManager = $this->createMock(IManager::class);
6564
$this->data = $this->createMock(Data::class);
66-
$this->data->method('send')
67-
->willReturn(1);
68-
6965
$this->userSettings = $this->getMockBuilder(UserSettings::class)
7066
->onlyMethods(['getUserSetting'])
7167
->disableOriginalConstructor()
7268
->getMock();
73-
7469
$l10n = $this->createMock(IL10N::class);
75-
7670
$this->notificationGenerator = $this->createMock(NotificationGenerator::class);
77-
7871
$this->l10nFactory = $this->createMock(IFactory::class);
79-
$this->l10nFactory->expects($this->any())
72+
$this->userConfig = $this->createMock(IUserConfig::class);
73+
74+
$this->data->method('send')
75+
->willReturn(1);
76+
$this->l10nFactory
8077
->method('get')
8178
->with('activity')
8279
->willReturn($l10n);
83-
84-
$this->userSettings->expects($this->any())
80+
$this->userSettings
8581
->method('getUserSetting')
8682
->with($this->stringContains('affectedUser'), $this->anything(), $this->anything())
8783
->willReturnMap([
@@ -92,6 +88,16 @@ protected function setUp(): void {
9288
['affectedUser', 'setting', 'batchtime', 10],
9389
['affectedUser2', 'setting', 'batchtime', 10],
9490
]);
91+
92+
$this->consumer = new Consumer(
93+
$this->data,
94+
$this->activityManager,
95+
$this->userSettings,
96+
$this->notificationGenerator,
97+
$this->userConfig,
98+
);
99+
100+
$this->event = Server::get(IManager::class)->generateEvent();
95101
}
96102

97103
protected function tearDown(): void {
@@ -100,31 +106,29 @@ protected function tearDown(): void {
100106
}
101107

102108
protected function deleteTestActivities(): void {
103-
$query = \OC::$server->getDatabaseConnection()->getQueryBuilder();
109+
$query = Server::get(IDBConnection::class)->getQueryBuilder();
104110
$query->delete('activity')
105111
->where($query->expr()->eq(
106112
'app', $query->createNamedParameter('test')
107113
));
108-
$query->execute();
114+
$query->executeStatement();
109115

110-
$query = \OC::$server->getDatabaseConnection()->getQueryBuilder();
116+
$query = Server::get(IDBConnection::class)->getQueryBuilder();
111117
$query->delete('activity_mq')
112118
->where($query->expr()->eq(
113119
'amq_appid', $query->createNamedParameter('test')
114120
));
115-
$query->execute();
121+
$query->executeStatement();
116122
}
117123

118-
public function receiveData(): array {
124+
public static function receiveData(): array {
119125
return [
120126
['type', 'author', 'affectedUser', 'subject', 'affectedUser'],
121127
['type2', 'author', 'affectedUser', 'subject', false],
122-
123128
['type', 'author', 'affectedUser', 'subject_self', 'affectedUser'],
124129
['type', 'author', 'affectedUser2', 'subject_self', 'affectedUser2'],
125130
['type', 'author', 'affectedUser', 'subject2', 'affectedUser'],
126131
['type', 'author', 'affectedUser2', 'subject2', 'affectedUser2'],
127-
128132
['type', 'affectedUser', 'affectedUser', 'subject_self', 'affectedUser'],
129133
['type', 'affectedUser2', 'affectedUser2', 'subject_self', false],
130134
['type', 'affectedUser', 'affectedUser', 'subject2', 'affectedUser'],
@@ -141,9 +145,14 @@ public function receiveData(): array {
141145
* @param string $subject
142146
* @param array|false $expected
143147
*/
144-
public function testReceiveStream(string $type, string $author, string $affectedUser, string $subject): void {
145-
$consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->notificationGenerator);
146-
$event = \OC::$server->getActivityManager()->generateEvent();
148+
public function testReceiveStream(string $type, string $author, string $affectedUser, string $subject, $expected): void {
149+
$consumer = new Consumer($this->data,
150+
$this->activityManager,
151+
$this->userSettings,
152+
$this->notificationGenerator,
153+
$this->userConfig,
154+
);
155+
$event = Server::get(IManager::class)->generateEvent();
147156
$event->setApp('test')
148157
->setType($type)
149158
->setAffectedUser($affectedUser)
@@ -172,9 +181,7 @@ public function testReceiveStream(string $type, string $author, string $affected
172181
*/
173182
public function testReceiveEmail(string $type, string $author, string $affectedUser, string $subject, $expected): void {
174183
$time = time();
175-
$consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->notificationGenerator);
176-
$event = \OC::$server->getActivityManager()->generateEvent();
177-
$event->setApp('test')
184+
$this->event->setApp('test')
178185
->setType($type)
179186
->setAffectedUser($affectedUser)
180187
->setAuthor($author)
@@ -190,10 +197,10 @@ public function testReceiveEmail(string $type, string $author, string $affectedU
190197
} else {
191198
$this->data->expects($this->once())
192199
->method('storeMail')
193-
->with($event, $time + 10);
200+
->with($this->event, $time + 10);
194201
}
195202

196-
$consumer->receive($event);
203+
$this->consumer->receive($this->event);
197204
}
198205

199206
/**
@@ -206,9 +213,7 @@ public function testReceiveEmail(string $type, string $author, string $affectedU
206213
* @param array|false $expected
207214
*/
208215
public function testReceiveNotification(string $type, string $author, string $affectedUser, string $subject, $expected): void {
209-
$consumer = new Consumer($this->data, $this->activityManager, $this->userSettings, $this->notificationGenerator);
210-
$event = \OC::$server->getActivityManager()->generateEvent();
211-
$event->setApp('test')
216+
$this->event->setApp('test')
212217
->setType($type)
213218
->setAffectedUser($affectedUser)
214219
->setAuthor($author)
@@ -227,6 +232,62 @@ public function testReceiveNotification(string $type, string $author, string $af
227232
->method('sendNotificationForEvent');
228233
}
229234

230-
$consumer->receive($event);
235+
$this->consumer->receive($this->event);
236+
}
237+
238+
public function receiveBulkData(): array {
239+
return [
240+
'empty affected users' => ['type', 'author', 'subject', [], null, false],
241+
'empty activity ids' => ['type', 'author', 'subject', ['affectedUser', 'affectedUser1'], [], false],
242+
'notification for non-author user' => ['type', 'author', 'subject', ['affectedUser'], [1 => 'affectedUser'], true],
243+
'no notification when author is the affected user' => ['type', 'author', 'subject', ['author'], [1 => 'author'], false],
244+
];
231245
}
246+
247+
/**
248+
* @dataProvider receiveBulkData
249+
*/
250+
public function testBulkReceiveNotification(string $type, string $author, string $subject, array $affectedUsers, ?array $activityIds, bool $expectNotification): void {
251+
$this->event->setApp('activity')
252+
->setType($type)
253+
->setAuthor($author)
254+
->setTimestamp(time())
255+
->setSubject($subject, ['subjectParam1', 'subjectParam2'])
256+
->setMessage('message', ['messageParam1', 'messageParam2'])
257+
->setObject('', 0, 'file')
258+
->setLink('link');
259+
260+
$settings = $this->createMock(ActivitySettings::class);
261+
$settings->method('canChangeMail')->willReturn(false);
262+
$settings->method('isDefaultEnabledMail')->willReturn(false);
263+
$settings->method('canChangeNotification')->willReturn(true);
264+
265+
if (empty($affectedUsers)) {
266+
$this->data->expects($this->never())
267+
->method('bulkSend');
268+
$this->data->expects($this->never())
269+
->method('storeMail');
270+
$this->notificationGenerator->expects($this->never())
271+
->method('sendNotificationForEvent');
272+
} else {
273+
$this->data->expects($this->once())
274+
->method('bulkSend')
275+
->willReturn($activityIds ?? []);
276+
277+
if ($expectNotification) {
278+
$this->userConfig->method('getValuesByUsers')
279+
->willReturnCallback(function (string $app, string $key, mixed $type, array $users): array {
280+
return array_fill_keys($users, true);
281+
});
282+
$this->notificationGenerator->expects($this->atLeastOnce())
283+
->method('sendNotificationForEvent');
284+
} else {
285+
$this->notificationGenerator->expects($this->never())
286+
->method('sendNotificationForEvent');
287+
}
288+
}
289+
290+
$this->consumer->bulkReceive($this->event, $affectedUsers, $settings);
291+
}
292+
232293
}

0 commit comments

Comments
 (0)