Skip to content

Commit 23dbc96

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #39524 from nextcloud/backport/39264/stable25
[stable25] Add instance category while checking new updates
2 parents 7076240 + 44b443e commit 23dbc96

File tree

4 files changed

+86
-78
lines changed

4 files changed

+86
-78
lines changed

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

Lines changed: 18 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
use OC\Installer;
3131
use OC\Updater\VersionCheck;
3232
use OCP\App\IAppManager;
33-
use OCP\Http\Client\IClientService;
3433
use OCP\IConfig;
3534
use OCP\IGroup;
3635
use OCP\IGroupManager;
@@ -39,42 +38,33 @@
3938
class BackgroundJob extends TimedJob {
4039
protected $connectionNotifications = [3, 7, 14, 30];
4140

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;
41+
protected IConfig $config;
42+
protected IManager $notificationManager;
43+
protected IGroupManager $groupManager;
44+
protected IAppManager $appManager;
45+
protected Installer $installer;
46+
protected VersionCheck $versionCheck;
5947

6048
/** @var string[] */
6149
protected $users;
6250

63-
public function __construct(IConfig $config,
64-
IManager $notificationManager,
65-
IGroupManager $groupManager,
66-
IAppManager $appManager,
67-
IClientService $client,
68-
Installer $installer) {
51+
public function __construct(
52+
IConfig $config,
53+
IManager $notificationManager,
54+
IGroupManager $groupManager,
55+
IAppManager $appManager,
56+
Installer $installer,
57+
VersionCheck $versionCheck
58+
) {
6959
// Run once a day
7060
$this->setInterval(60 * 60 * 24);
7161

7262
$this->config = $config;
7363
$this->notificationManager = $notificationManager;
7464
$this->groupManager = $groupManager;
7565
$this->appManager = $appManager;
76-
$this->client = $client;
7766
$this->installer = $installer;
67+
$this->versionCheck = $versionCheck;
7868
}
7969

8070
protected function run($argument) {
@@ -101,12 +91,10 @@ protected function checkCoreUpdate() {
10191
return;
10292
}
10393

104-
$updater = $this->createVersionCheck();
105-
106-
$status = $updater->check();
94+
$status = $this->versionCheck->check();
10795
if ($status === false) {
108-
$errors = 1 + (int) $this->config->getAppValue('updatenotification', 'update_check_errors', 0);
109-
$this->config->setAppValue('updatenotification', 'update_check_errors', $errors);
96+
$errors = 1 + (int) $this->config->getAppValue('updatenotification', 'update_check_errors', '0');
97+
$this->config->setAppValue('updatenotification', 'update_check_errors', (string) $errors);
11098

11199
if (\in_array($errors, $this->connectionNotifications, true)) {
112100
$this->sendErrorNotifications($errors);
@@ -255,16 +243,6 @@ protected function deleteOutdatedNotifications($app, $version) {
255243
$this->notificationManager->markProcessed($notification);
256244
}
257245

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

‎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: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,27 +28,28 @@
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) {
36+
private IClientService $clientService;
37+
private IConfig $config;
38+
private IUserManager $userManager;
39+
private IRegistry $registry;
40+
41+
public function __construct(
42+
IClientService $clientService,
43+
IConfig $config,
44+
IUserManager $userManager,
45+
IRegistry $registry
46+
) {
4747
$this->clientService = $clientService;
4848
$this->config = $config;
49+
$this->userManager = $userManager;
50+
$this->registry = $registry;
4951
}
5052

51-
5253
/**
5354
* Check if a new version is available
5455
*
@@ -82,6 +83,8 @@ public function check() {
8283
$version['php_major'] = PHP_MAJOR_VERSION;
8384
$version['php_minor'] = PHP_MINOR_VERSION;
8485
$version['php_release'] = PHP_RELEASE_VERSION;
86+
$version['category'] = $this->computeCategory();
87+
$version['isSubscriber'] = (int) $this->registry->delegateHasValidSubscription();
8588
$versionString = implode('x', $version);
8689

8790
//fetch xml data from updater
@@ -131,4 +134,25 @@ protected function getUrlContent($url) {
131134
$response = $client->get($url);
132135
return $response->getBody();
133136
}
137+
138+
private function computeCategory(): int {
139+
$categoryBoundaries = [
140+
100,
141+
500,
142+
1000,
143+
5000,
144+
10000,
145+
100000,
146+
1000000,
147+
];
148+
149+
$nbUsers = $this->userManager->countSeenUsers();
150+
foreach ($categoryBoundaries as $categoryId => $boundary) {
151+
if ($nbUsers <= $boundary) {
152+
return $categoryId;
153+
}
154+
}
155+
156+
return count($categoryBoundaries);
157+
}
134158
}

‎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)