Merge pull request #39523 from nextcloud/backport/add-instance-category

[stable26] Add instance category while checking new updates
nextcloud · Jul 24, 2023 · 3c3befe · 3c3befe
2 parents 57b0bf3 + 25ba08b
commit 3c3befe
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 88 deletions.
diff --git a/apps/updatenotification/lib/Notification/BackgroundJob.php b/apps/updatenotification/lib/Notification/BackgroundJob.php
@@ -31,7 +31,6 @@
 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 @@
 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 @@ protected function checkCoreUpdate() {
 			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', $errors);
+			$errors = 1 + (int) $this->config->getAppValue('updatenotification', 'update_check_errors', '0');
+			$this->config->setAppValue('updatenotification', 'update_check_errors', (string) $errors);
 
 			if (\in_array($errors, $this->connectionNotifications, true)) {
 				$this->sendErrorNotifications($errors);
@@ -258,16 +232,6 @@ protected function deleteOutdatedNotifications($app, $version) {
 		$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
@@ -32,7 +32,6 @@
 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 @@ protected function setUp(): void {
 		$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 @@ protected function getJob(array $methods = []) {
 				$this->notificationManager,
 				$this->groupManager,
 				$this->appManager,
-				$this->client,
-				$this->installer
+				$this->installer,
+				$this->versionCheck,
 			);
 		}
 		{
@@ -94,8 +93,8 @@ protected function getJob(array $methods = []) {
 					$this->notificationManager,
 					$this->groupManager,
 					$this->appManager,
-					$this->client,
 					$this->installer,
+					$this->versionCheck,
 				])
 				->setMethods($methods)
 				->getMock();
@@ -160,7 +159,6 @@ public function dataCheckCoreUpdate(): array {
 	public function testCheckCoreUpdate(string $channel, $versionCheck, $version, $readableVersion, $errorDays) {
 		$job = $this->getJob([
 			'getChannel',
-			'createVersionCheck',
 			'createNotifications',
 			'clearErrorNotifications',
 			'sendErrorNotifications',
@@ -171,17 +169,12 @@ public function testCheckCoreUpdate(string $channel, $versionCheck, $version, $r
 			->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
@@ -28,23 +28,17 @@
 
 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 @@ public function check() {
 		$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 @@ protected function getUrlContent($url) {
 		$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
@@ -25,13 +25,17 @@
 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 {
 	/** @var IConfig| \PHPUnit\Framework\MockObject\MockObject */
 	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 @@ protected function setUp(): void {
 			->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 @@ protected function setUp(): void {
 	 * @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 @@ public function testCheckWithoutUpdateUrl() {
 				0,
 				'installedat',
 				'installedat',
-				'lastupdatedat'
+				'lastupdatedat',
 			);
 		$this->config
 			->expects($this->once())
@@ -166,7 +179,7 @@ public function testCheckWithInvalidXml() {
 				0,
 				'installedat',
 				'installedat',
-				'lastupdatedat'
+				'lastupdatedat',
 			);
 		$this->config
 			->expects($this->once())
@@ -220,7 +233,7 @@ public function testCheckWithEmptyValidXmlResponse() {
 				0,
 				'installedat',
 				'installedat',
-				'lastupdatedat'
+				'lastupdatedat',
 			);
 		$this->config
 			->expects($this->once())
@@ -273,7 +286,7 @@ public function testCheckWithEmptyInvalidXmlResponse() {
 				0,
 				'installedat',
 				'installedat',
-				'lastupdatedat'
+				'lastupdatedat',
 			);
 		$this->config
 			->expects($this->once())
@@ -327,7 +340,7 @@ public function testCheckWithMissingAttributeXmlResponse() {
 				0,
 				'installedat',
 				'installedat',
-				'lastupdatedat'
+				'lastupdatedat',
 			);
 		$this->config
 			->expects($this->once())