From 31f135477792902c3b7380fba6406202e7815ca1 Mon Sep 17 00:00:00 2001 From: Benjamin Gaussorgues Date: Fri, 7 Jul 2023 09:51:08 +0200 Subject: [PATCH] Add instance category while checking new updates Signed-off-by: Benjamin Gaussorgues --- .../lib/Notification/BackgroundJob.php | 58 ++++--------------- .../tests/Notification/BackgroundJobTest.php | 31 ++++------ lib/private/Updater/VersionCheck.php | 45 +++++++++----- tests/lib/Updater/VersionCheckTest.php | 27 ++++++--- 4 files changed, 74 insertions(+), 87 deletions(-) diff --git a/apps/updatenotification/lib/Notification/BackgroundJob.php b/apps/updatenotification/lib/Notification/BackgroundJob.php index f8f1f41e589..4889c931238 100644 --- a/apps/updatenotification/lib/Notification/BackgroundJob.php +++ b/apps/updatenotification/lib/Notification/BackgroundJob.php @@ -31,7 +31,6 @@ use OC\Updater\VersionCheck; use OCP\App\IAppManager; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; -use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; @@ -40,44 +39,21 @@ use OCP\Notification\IManager; class BackgroundJob extends TimedJob { protected $connectionNotifications = [3, 7, 14, 30]; - /** @var IConfig */ - protected $config; - - /** @var IManager */ - protected $notificationManager; - - /** @var IGroupManager */ - protected $groupManager; - - /** @var IAppManager */ - protected $appManager; - - /** @var IClientService */ - protected $client; - - /** @var Installer */ - protected $installer; - /** @var string[] */ protected $users; - public function __construct(ITimeFactory $timeFactory, - IConfig $config, - IManager $notificationManager, - IGroupManager $groupManager, - IAppManager $appManager, - IClientService $client, - Installer $installer) { + public function __construct( + ITimeFactory $timeFactory, + protected IConfig $config, + protected IManager $notificationManager, + protected IGroupManager $groupManager, + protected IAppManager $appManager, + protected Installer $installer, + protected VersionCheck $versionCheck, + ) { parent::__construct($timeFactory); // Run once a day $this->setInterval(60 * 60 * 24); - - $this->config = $config; - $this->notificationManager = $notificationManager; - $this->groupManager = $groupManager; - $this->appManager = $appManager; - $this->client = $client; - $this->installer = $installer; } protected function run($argument) { @@ -104,12 +80,10 @@ class BackgroundJob extends TimedJob { return; } - $updater = $this->createVersionCheck(); - - $status = $updater->check(); + $status = $this->versionCheck->check(); if ($status === false) { $errors = 1 + (int) $this->config->getAppValue('updatenotification', 'update_check_errors', '0'); - $this->config->setAppValue('updatenotification', 'update_check_errors', (string)$errors); + $this->config->setAppValue('updatenotification', 'update_check_errors', (string) $errors); if (\in_array($errors, $this->connectionNotifications, true)) { $this->sendErrorNotifications($errors); @@ -258,16 +232,6 @@ class BackgroundJob extends TimedJob { $this->notificationManager->markProcessed($notification); } - /** - * @return VersionCheck - */ - protected function createVersionCheck(): VersionCheck { - return new VersionCheck( - $this->client, - $this->config - ); - } - /** * @return string */ diff --git a/apps/updatenotification/tests/Notification/BackgroundJobTest.php b/apps/updatenotification/tests/Notification/BackgroundJobTest.php index 70d1a918a01..df8b104e9ca 100644 --- a/apps/updatenotification/tests/Notification/BackgroundJobTest.php +++ b/apps/updatenotification/tests/Notification/BackgroundJobTest.php @@ -32,7 +32,6 @@ use OC\Updater\VersionCheck; use OCA\UpdateNotification\Notification\BackgroundJob; use OCP\App\IAppManager; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; @@ -51,12 +50,12 @@ class BackgroundJobTest extends TestCase { protected $groupManager; /** @var IAppManager|MockObject */ protected $appManager; - /** @var IClientService|MockObject */ - protected $client; - /** @var Installer|MockObject */ - protected $installer; /** @var ITimeFactory|MockObject */ protected $timeFactory; + /** @var Installer|MockObject */ + protected $installer; + /** @var VersionCheck|MockObject */ + protected $versionCheck; protected function setUp(): void { parent::setUp(); @@ -65,9 +64,9 @@ class BackgroundJobTest extends TestCase { $this->notificationManager = $this->createMock(IManager::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->appManager = $this->createMock(IAppManager::class); - $this->client = $this->createMock(IClientService::class); - $this->installer = $this->createMock(Installer::class); $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->installer = $this->createMock(Installer::class); + $this->versionCheck = $this->createMock(VersionCheck::class); } /** @@ -82,8 +81,8 @@ class BackgroundJobTest extends TestCase { $this->notificationManager, $this->groupManager, $this->appManager, - $this->client, - $this->installer + $this->installer, + $this->versionCheck, ); } { @@ -94,8 +93,8 @@ class BackgroundJobTest extends TestCase { $this->notificationManager, $this->groupManager, $this->appManager, - $this->client, $this->installer, + $this->versionCheck, ]) ->setMethods($methods) ->getMock(); @@ -160,7 +159,6 @@ class BackgroundJobTest extends TestCase { public function testCheckCoreUpdate(string $channel, $versionCheck, $version, $readableVersion, $errorDays) { $job = $this->getJob([ 'getChannel', - 'createVersionCheck', 'createNotifications', 'clearErrorNotifications', 'sendErrorNotifications', @@ -171,17 +169,12 @@ class BackgroundJobTest extends TestCase { ->willReturn($channel); if ($versionCheck === null) { - $job->expects($this->never()) - ->method('createVersionCheck'); + $this->versionCheck->expects($this->never()) + ->method('check'); } else { - $check = $this->createMock(VersionCheck::class); - $check->expects($this->once()) + $this->versionCheck->expects($this->once()) ->method('check') ->willReturn($versionCheck); - - $job->expects($this->once()) - ->method('createVersionCheck') - ->willReturn($check); } if ($version === null) { diff --git a/lib/private/Updater/VersionCheck.php b/lib/private/Updater/VersionCheck.php index a634ae4cc71..2aab260716a 100644 --- a/lib/private/Updater/VersionCheck.php +++ b/lib/private/Updater/VersionCheck.php @@ -28,23 +28,17 @@ namespace OC\Updater; use OCP\Http\Client\IClientService; use OCP\IConfig; +use OCP\IUserManager; +use OCP\Support\Subscription\IRegistry; use OCP\Util; class VersionCheck { - /** @var IClientService */ - private $clientService; - - /** @var IConfig */ - private $config; - - /** - * @param IClientService $clientService - * @param IConfig $config - */ - public function __construct(IClientService $clientService, - IConfig $config) { - $this->clientService = $clientService; - $this->config = $config; + public function __construct( + private IClientService $clientService, + private IConfig $config, + private IUserManager $userManager, + private IRegistry $registry, + ) { } @@ -81,6 +75,8 @@ class VersionCheck { $version['php_major'] = PHP_MAJOR_VERSION; $version['php_minor'] = PHP_MINOR_VERSION; $version['php_release'] = PHP_RELEASE_VERSION; + $version['category'] = $this->computeCategory(); + $version['isSubscriber'] = (int) $this->registry->delegateHasValidSubscription(); $versionString = implode('x', $version); //fetch xml data from updater @@ -130,4 +126,25 @@ class VersionCheck { $response = $client->get($url); return $response->getBody(); } + + private function computeCategory(): int { + $categoryBoundaries = [ + 100, + 500, + 1000, + 5000, + 10000, + 100000, + 1000000, + ]; + + $nbUsers = $this->userManager->countSeenUsers(); + foreach ($categoryBoundaries as $categoryId => $boundary) { + if ($nbUsers <= $boundary) { + return $categoryId; + } + } + + return count($categoryBoundaries); + } } diff --git a/tests/lib/Updater/VersionCheckTest.php b/tests/lib/Updater/VersionCheckTest.php index 1cd632875c3..be847253035 100644 --- a/tests/lib/Updater/VersionCheckTest.php +++ b/tests/lib/Updater/VersionCheckTest.php @@ -25,6 +25,8 @@ namespace Test\Updater; use OC\Updater\VersionCheck; use OCP\Http\Client\IClientService; use OCP\IConfig; +use OCP\IUserManager; +use OCP\Support\Subscription\IRegistry; use OCP\Util; class VersionCheckTest extends \Test\TestCase { @@ -32,6 +34,8 @@ class VersionCheckTest extends \Test\TestCase { private $config; /** @var VersionCheck | \PHPUnit\Framework\MockObject\MockObject*/ private $updater; + /** @var IRegistry | \PHPUnit\Framework\Mo2ckObject\MockObject*/ + private $registry; protected function setUp(): void { parent::setUp(); @@ -42,9 +46,18 @@ class VersionCheckTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); + $this->registry = $this->createMock(IRegistry::class); + $this->registry + ->method('delegateHasValidSubscription') + ->willReturn(false); $this->updater = $this->getMockBuilder(VersionCheck::class) ->setMethods(['getUrlContent']) - ->setConstructorArgs([$clientService, $this->config]) + ->setConstructorArgs([ + $clientService, + $this->config, + $this->createMock(IUserManager::class), + $this->registry, + ]) ->getMock(); } @@ -53,7 +66,7 @@ class VersionCheckTest extends \Test\TestCase { * @return string */ private function buildUpdateUrl($baseUrl) { - return $baseUrl . '?version='.implode('x', Util::getVersion()).'xinstalledatxlastupdatedatx'.\OC_Util::getChannel().'xxx'.PHP_MAJOR_VERSION.'x'.PHP_MINOR_VERSION.'x'.PHP_RELEASE_VERSION; + return $baseUrl . '?version='.implode('x', Util::getVersion()).'xinstalledatxlastupdatedatx'.\OC_Util::getChannel().'xxx'.PHP_MAJOR_VERSION.'x'.PHP_MINOR_VERSION.'x'.PHP_RELEASE_VERSION.'x0x0'; } public function testCheckInCache() { @@ -114,7 +127,7 @@ class VersionCheckTest extends \Test\TestCase { '0', 'installedat', 'installedat', - 'lastupdatedat' + 'lastupdatedat', ); $this->config ->expects($this->once()) @@ -166,7 +179,7 @@ class VersionCheckTest extends \Test\TestCase { '0', 'installedat', 'installedat', - 'lastupdatedat' + 'lastupdatedat', ); $this->config ->expects($this->once()) @@ -220,7 +233,7 @@ class VersionCheckTest extends \Test\TestCase { '0', 'installedat', 'installedat', - 'lastupdatedat' + 'lastupdatedat', ); $this->config ->expects($this->once()) @@ -273,7 +286,7 @@ class VersionCheckTest extends \Test\TestCase { '0', 'installedat', 'installedat', - 'lastupdatedat' + 'lastupdatedat', ); $this->config ->expects($this->once()) @@ -327,7 +340,7 @@ class VersionCheckTest extends \Test\TestCase { '0', 'installedat', 'installedat', - 'lastupdatedat' + 'lastupdatedat', ); $this->config ->expects($this->once()) -- 2.39.5