Skip to content

Commit 091ecae

Browse files
AltahrimAltahrim
committed
Add instance category while checking new updates
Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
1 parent a593eb2 commit 091ecae

4 files changed

Lines changed: 73 additions & 87 deletions

File tree

‎apps/updatenotification/lib/Notification/BackgroundJob.php‎

Lines changed: 12 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
use OC\Installer;
3131
use OC\Updater\VersionCheck;
3232
use OCP\App\IAppManager;
33-
use OCP\Http\Client\IClientService;
33+
use OCP\BackgroundJob\TimedJob;
3434
use OCP\IConfig;
3535
use OCP\IGroup;
3636
use OCP\IGroupManager;
@@ -39,42 +39,19 @@
3939
class BackgroundJob extends TimedJob {
4040
protected $connectionNotifications = [3, 7, 14, 30];
4141

42-
/** @var IConfig */
43-
protected $config;
44-
45-
/** @var IManager */
46-
protected $notificationManager;
47-
48-
/** @var IGroupManager */
49-
protected $groupManager;
50-
51-
/** @var IAppManager */
52-
protected $appManager;
53-
54-
/** @var IClientService */
55-
protected $client;
56-
57-
/** @var Installer */
58-
protected $installer;
59-
6042
/** @var string[] */
6143
protected $users;
6244

63-
public function __construct(IConfig $config,
64-
IManager $notificationManager,
65-
IGroupManager $groupManager,
66-
IAppManager $appManager,
67-
IClientService $client,
68-
Installer $installer) {
45+
public function __construct(
46+
protected IConfig $config,
47+
protected IManager $notificationManager,
48+
protected IGroupManager $groupManager,
49+
protected IAppManager $appManager,
50+
protected Installer $installer,
51+
protected VersionCheck $versionCheck,
52+
) {
6953
// Run once a day
7054
$this->setInterval(60 * 60 * 24);
71-
72-
$this->config = $config;
73-
$this->notificationManager = $notificationManager;
74-
$this->groupManager = $groupManager;
75-
$this->appManager = $appManager;
76-
$this->client = $client;
77-
$this->installer = $installer;
7855
}
7956

8057
protected function run($argument) {
@@ -101,12 +78,10 @@ protected function checkCoreUpdate() {
10178
return;
10279
}
10380

104-
$updater = $this->createVersionCheck();
105-
106-
$status = $updater->check();
81+
$status = $this->versionCheck->check();
10782
if ($status === false) {
108-
$errors = 1 + (int) $this->config->getAppValue('updatenotification', 'update_check_errors', 0);
109-
$this->config->setAppValue('updatenotification', 'update_check_errors', $errors);
83+
$errors = 1 + (int) $this->config->getAppValue('updatenotification', 'update_check_errors', '0');
84+
$this->config->setAppValue('updatenotification', 'update_check_errors', (string) $errors);
11085

11186
if (\in_array($errors, $this->connectionNotifications, true)) {
11287
$this->sendErrorNotifications($errors);
@@ -255,16 +230,6 @@ protected function deleteOutdatedNotifications($app, $version) {
255230
$this->notificationManager->markProcessed($notification);
256231
}
257232

258-
/**
259-
* @return VersionCheck
260-
*/
261-
protected function createVersionCheck(): VersionCheck {
262-
return new VersionCheck(
263-
$this->client,
264-
$this->config
265-
);
266-
}
267-
268233
/**
269234
* @return string
270235
*/

‎apps/updatenotification/tests/Notification/BackgroundJobTest.php‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
use OC\Updater\VersionCheck;
3232
use OCA\UpdateNotification\Notification\BackgroundJob;
3333
use OCP\App\IAppManager;
34-
use OCP\Http\Client\IClientService;
3534
use OCP\IConfig;
3635
use OCP\IGroup;
3736
use OCP\IGroupManager;
@@ -50,10 +49,10 @@ class BackgroundJobTest extends TestCase {
5049
protected $groupManager;
5150
/** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject */
5251
protected $appManager;
53-
/** @var IClientService|\PHPUnit\Framework\MockObject\MockObject */
54-
protected $client;
55-
/** @var Installer|\PHPUnit\Framework\MockObject\MockObject */
52+
/** @var Installer|MockObject */
5653
protected $installer;
54+
/** @var VersionCheck|MockObject */
55+
protected $versionCheck;
5756

5857
protected function setUp(): void {
5958
parent::setUp();
@@ -62,8 +61,8 @@ protected function setUp(): void {
6261
$this->notificationManager = $this->createMock(IManager::class);
6362
$this->groupManager = $this->createMock(IGroupManager::class);
6463
$this->appManager = $this->createMock(IAppManager::class);
65-
$this->client = $this->createMock(IClientService::class);
6664
$this->installer = $this->createMock(Installer::class);
65+
$this->versionCheck = $this->createMock(VersionCheck::class);
6766
}
6867

6968
/**
@@ -77,8 +76,8 @@ protected function getJob(array $methods = []) {
7776
$this->notificationManager,
7877
$this->groupManager,
7978
$this->appManager,
80-
$this->client,
81-
$this->installer
79+
$this->installer,
80+
$this->versionCheck,
8281
);
8382
}
8483
{
@@ -88,8 +87,8 @@ protected function getJob(array $methods = []) {
8887
$this->notificationManager,
8988
$this->groupManager,
9089
$this->appManager,
91-
$this->client,
9290
$this->installer,
91+
$this->versionCheck,
9392
])
9493
->setMethods($methods)
9594
->getMock();
@@ -154,7 +153,6 @@ public function dataCheckCoreUpdate(): array {
154153
public function testCheckCoreUpdate(string $channel, $versionCheck, $version, $readableVersion, $errorDays) {
155154
$job = $this->getJob([
156155
'getChannel',
157-
'createVersionCheck',
158156
'createNotifications',
159157
'clearErrorNotifications',
160158
'sendErrorNotifications',
@@ -165,17 +163,12 @@ public function testCheckCoreUpdate(string $channel, $versionCheck, $version, $r
165163
->willReturn($channel);
166164

167165
if ($versionCheck === null) {
168-
$job->expects($this->never())
169-
->method('createVersionCheck');
166+
$this->versionCheck->expects($this->never())
167+
->method('check');
170168
} else {
171-
$check = $this->createMock(VersionCheck::class);
172-
$check->expects($this->once())
169+
$this->versionCheck->expects($this->once())
173170
->method('check')
174171
->willReturn($versionCheck);
175-
176-
$job->expects($this->once())
177-
->method('createVersionCheck')
178-
->willReturn($check);
179172
}
180173

181174
if ($version === null) {

‎lib/private/Updater/VersionCheck.php‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,27 +28,19 @@
2828

2929
use OCP\Http\Client\IClientService;
3030
use OCP\IConfig;
31+
use OCP\IUserManager;
32+
use OCP\Support\Subscription\IRegistry;
3133
use OCP\Util;
3234

3335
class VersionCheck {
34-
35-
/** @var IClientService */
36-
private $clientService;
37-
38-
/** @var IConfig */
39-
private $config;
40-
41-
/**
42-
* @param IClientService $clientService
43-
* @param IConfig $config
44-
*/
45-
public function __construct(IClientService $clientService,
46-
IConfig $config) {
47-
$this->clientService = $clientService;
48-
$this->config = $config;
36+
public function __construct(
37+
private IClientService $clientService,
38+
private IConfig $config,
39+
private IUserManager $userManager,
40+
private IRegistry $registry,
41+
) {
4942
}
5043

51-
5244
/**
5345
* Check if a new version is available
5446
*
@@ -82,6 +74,8 @@ public function check() {
8274
$version['php_major'] = PHP_MAJOR_VERSION;
8375
$version['php_minor'] = PHP_MINOR_VERSION;
8476
$version['php_release'] = PHP_RELEASE_VERSION;
77+
$version['category'] = $this->computeCategory();
78+
$version['isSubscriber'] = (int) $this->registry->delegateHasValidSubscription();
8579
$versionString = implode('x', $version);
8680

8781
//fetch xml data from updater
@@ -131,4 +125,25 @@ protected function getUrlContent($url) {
131125
$response = $client->get($url);
132126
return $response->getBody();
133127
}
128+
129+
private function computeCategory(): int {
130+
$categoryBoundaries = [
131+
100,
132+
500,
133+
1000,
134+
5000,
135+
10000,
136+
100000,
137+
1000000,
138+
];
139+
140+
$nbUsers = $this->userManager->countSeenUsers();
141+
foreach ($categoryBoundaries as $categoryId => $boundary) {
142+
if ($nbUsers <= $boundary) {
143+
return $categoryId;
144+
}
145+
}
146+
147+
return count($categoryBoundaries);
148+
}
134149
}

‎tests/lib/Updater/VersionCheckTest.php‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,17 @@
2525
use OC\Updater\VersionCheck;
2626
use OCP\Http\Client\IClientService;
2727
use OCP\IConfig;
28+
use OCP\IUserManager;
29+
use OCP\Support\Subscription\IRegistry;
2830
use OCP\Util;
2931

3032
class VersionCheckTest extends \Test\TestCase {
3133
/** @var IConfig| \PHPUnit\Framework\MockObject\MockObject */
3234
private $config;
3335
/** @var VersionCheck | \PHPUnit\Framework\MockObject\MockObject*/
3436
private $updater;
37+
/** @var IRegistry | \PHPUnit\Framework\Mo2ckObject\MockObject*/
38+
private $registry;
3539

3640
protected function setUp(): void {
3741
parent::setUp();
@@ -42,9 +46,18 @@ protected function setUp(): void {
4246
->disableOriginalConstructor()
4347
->getMock();
4448

49+
$this->registry = $this->createMock(IRegistry::class);
50+
$this->registry
51+
->method('delegateHasValidSubscription')
52+
->willReturn(false);
4553
$this->updater = $this->getMockBuilder(VersionCheck::class)
4654
->setMethods(['getUrlContent'])
47-
->setConstructorArgs([$clientService, $this->config])
55+
->setConstructorArgs([
56+
$clientService,
57+
$this->config,
58+
$this->createMock(IUserManager::class),
59+
$this->registry,
60+
])
4861
->getMock();
4962
}
5063

@@ -53,7 +66,7 @@ protected function setUp(): void {
5366
* @return string
5467
*/
5568
private function buildUpdateUrl($baseUrl) {
56-
return $baseUrl . '?version='.implode('x', Util::getVersion()).'xinstalledatxlastupdatedatx'.\OC_Util::getChannel().'xxx'.PHP_MAJOR_VERSION.'x'.PHP_MINOR_VERSION.'x'.PHP_RELEASE_VERSION;
69+
return $baseUrl . '?version='.implode('x', Util::getVersion()).'xinstalledatxlastupdatedatx'.\OC_Util::getChannel().'xxx'.PHP_MAJOR_VERSION.'x'.PHP_MINOR_VERSION.'x'.PHP_RELEASE_VERSION.'x0x0';
5770
}
5871

5972
public function testCheckInCache() {
@@ -114,7 +127,7 @@ public function testCheckWithoutUpdateUrl() {
114127
0,
115128
'installedat',
116129
'installedat',
117-
'lastupdatedat'
130+
'lastupdatedat',
118131
);
119132
$this->config
120133
->expects($this->once())
@@ -166,7 +179,7 @@ public function testCheckWithInvalidXml() {
166179
0,
167180
'installedat',
168181
'installedat',
169-
'lastupdatedat'
182+
'lastupdatedat',
170183
);
171184
$this->config
172185
->expects($this->once())
@@ -220,7 +233,7 @@ public function testCheckWithEmptyValidXmlResponse() {
220233
0,
221234
'installedat',
222235
'installedat',
223-
'lastupdatedat'
236+
'lastupdatedat',
224237
);
225238
$this->config
226239
->expects($this->once())
@@ -273,7 +286,7 @@ public function testCheckWithEmptyInvalidXmlResponse() {
273286
0,
274287
'installedat',
275288
'installedat',
276-
'lastupdatedat'
289+
'lastupdatedat',
277290
);
278291
$this->config
279292
->expects($this->once())
@@ -327,7 +340,7 @@ public function testCheckWithMissingAttributeXmlResponse() {
327340
0,
328341
'installedat',
329342
'installedat',
330-
'lastupdatedat'
343+
'lastupdatedat',
331344
);
332345
$this->config
333346
->expects($this->once())

0 commit comments

Comments
 (0)