Merge pull request #31537 from nextcloud/enh/31533/disable-webupdater…

…-biginstances show that the web updater is not recommended on big instances
nextcloud · Mar 28, 2022 · 8acefc9 · 8acefc9
2 parents 39494fb + 4e11d7d
commit 8acefc9
Show file tree

Hide file tree

Showing 5 changed files with 176 additions and 5 deletions.
diff --git a/apps/updatenotification/lib/Settings/Admin.php b/apps/updatenotification/lib/Settings/Admin.php
@@ -29,6 +29,8 @@
  */
 namespace OCA\UpdateNotification\Settings;
 
+use OC\User\Backend;
+use OCP\User\Backend\ICountUsersBackend;
 use OCA\UpdateNotification\UpdateChecker;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\IConfig;
@@ -38,6 +40,8 @@
 use OCP\Settings\ISettings;
 use OCP\Support\Subscription\IRegistry;
 use OCP\Util;
+use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 
 class Admin implements ISettings {
 	/** @var IConfig */
@@ -52,21 +56,29 @@ class Admin implements ISettings {
 	private $l10nFactory;
 	/** @var IRegistry */
 	private $subscriptionRegistry;
+	/** @var IUserManager */
+	private $userManager;
+	/** @var LoggerInterface */
+	private $logger;
 
 	public function __construct(
 		IConfig $config,
 		UpdateChecker $updateChecker,
 		IGroupManager $groupManager,
 		IDateTimeFormatter $dateTimeFormatter,
 		IFactory $l10nFactory,
-		IRegistry $subscriptionRegistry
+		IRegistry $subscriptionRegistry,
+		IUserManager $userManager,
+		LoggerInterface $logger
 	) {
 		$this->config = $config;
 		$this->updateChecker = $updateChecker;
 		$this->groupManager = $groupManager;
 		$this->dateTimeFormatter = $dateTimeFormatter;
 		$this->l10nFactory = $l10nFactory;
 		$this->subscriptionRegistry = $subscriptionRegistry;
+		$this->userManager = $userManager;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -111,6 +123,7 @@ public function getForm(): TemplateResponse {
 			'downloadLink' => empty($updateState['downloadLink']) ? '' : $updateState['downloadLink'],
 			'changes' => $this->filterChanges($updateState['changes'] ?? []),
 			'webUpdaterEnabled' => !$this->config->getSystemValue('upgrade.disable-web', false),
+			'isWebUpdaterRecommended' => $this->isWebUpdaterRecommended(),
 			'updaterEnabled' => empty($updateState['updaterEnabled']) ? false : $updateState['updaterEnabled'],
 			'versionIsEol' => empty($updateState['versionIsEol']) ? false : $updateState['versionIsEol'],
 			'isDefaultUpdateServerURL' => $isDefaultUpdateServerURL,
@@ -184,4 +197,28 @@ public function getSection(): string {
 	public function getPriority(): int {
 		return 11;
 	}
+
+	private function isWebUpdaterRecommended(): bool {
+		return $this->getUserCount() < 100;
+	}
+
+	/**
+	 * @see https://github.com/nextcloud/server/blob/39494fbf794d982f6f6551c984e6ca4c4e947d01/lib/private/Support/Subscription/Registry.php#L188-L216 implementation reference
+	 */
+	private function getUserCount(): int {
+		$userCount = 0;
+		$backends = $this->userManager->getBackends();
+		foreach ($backends as $backend) {
+			// TODO: change below to 'if ($backend instanceof ICountUsersBackend) {'
+			if ($backend->implementsActions(Backend::COUNT_USERS)) {
+				/** @var ICountUsersBackend $backend */
+				$backendUsers = $backend->countUsers();
+				if ($backendUsers !== false) {
+					$userCount += $backendUsers;
+				}
+			}
+		}
+
+		return $userCount;
+	}
 }
diff --git a/apps/updatenotification/src/components/UpdateNotification.vue b/apps/updatenotification/src/components/UpdateNotification.vue
@@ -41,6 +41,12 @@
 					</ul>
 				</template>
 
+				<template v-if="!isWebUpdaterRecommended && updaterEnabled && webUpdaterEnabled">
+					<h3 class="warning">
+						{{ t('updatenotification', 'Please note that the web updater is not recommended with more than 100 users! Please use the command line updater instead!') }}
+					</h3>
+				</template>
+
 				<div>
 					<a v-if="updaterEnabled && webUpdaterEnabled"
 						href="#"
@@ -136,6 +142,7 @@ export default {
 			lastCheckedDate: '',
 			isUpdateChecked: false,
 			webUpdaterEnabled: true,
+			isWebUpdaterRecommended: true,
 			updaterEnabled: true,
 			versionIsEol: false,
 			downloadLink: '',
@@ -328,6 +335,7 @@ export default {
 		this.lastCheckedDate = data.lastChecked
 		this.isUpdateChecked = data.isUpdateChecked
 		this.webUpdaterEnabled = data.webUpdaterEnabled
+		this.isWebUpdaterRecommended = data.isWebUpdaterRecommended
 		this.updaterEnabled = data.updaterEnabled
 		this.downloadLink = data.downloadLink
 		this.isNewVersionAvailable = data.isNewVersionAvailable

diff --git a/apps/updatenotification/tests/Settings/AdminTest.php b/apps/updatenotification/tests/Settings/AdminTest.php
@@ -41,6 +41,8 @@
 use OCP\Support\Subscription\IRegistry;
 use OCP\Util;
 use Test\TestCase;
+use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 
 class AdminTest extends TestCase {
 	/** @var IFactory|\PHPUnit\Framework\MockObject\MockObject */
@@ -57,6 +59,10 @@ class AdminTest extends TestCase {
 	private $dateTimeFormatter;
 	/** @var IRegistry|\PHPUnit\Framework\MockObject\MockObject */
 	private $subscriptionRegistry;
+	/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
+	private $userManager;
+	/** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
+	private $logger;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -67,13 +73,58 @@ protected function setUp(): void {
 		$this->dateTimeFormatter = $this->createMock(IDateTimeFormatter::class);
 		$this->l10nFactory = $this->createMock(IFactory::class);
 		$this->subscriptionRegistry = $this->createMock(IRegistry::class);
+		$this->userManager = $this->createMock(IUserManager::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 
 		$this->admin = new Admin(
-			$this->config, $this->updateChecker, $this->groupManager, $this->dateTimeFormatter, $this->l10nFactory, $this->subscriptionRegistry
+			$this->config, 
+			$this->updateChecker, 
+			$this->groupManager, 
+			$this->dateTimeFormatter, 
+			$this->l10nFactory, 
+			$this->subscriptionRegistry,
+			$this->userManager,
+			$this->logger
 		);
 	}
 
 	public function testGetFormWithUpdate() {
+		$backend1 = $this->createMock(UserInterface::class);
+		$backend2 = $this->createMock(UserInterface::class);
+		$backend3 = $this->createMock(UserInterface::class);
+		$backend1
+			->expects($this->once())
+			->method('implementsActions')
+			->with(Backend::COUNT_USERS)
+			->willReturn(false);
+		$backend2
+			->expects($this->once())
+			->method('implementsActions')
+			->with(Backend::COUNT_USERS)
+			->willReturn(true);
+		$backend3
+			->expects($this->once())
+			->method('implementsActions')
+			->with(Backend::COUNT_USERS)
+			->willReturn(true);
+		$backend1
+			->expects($this->never())
+			->method('countUsers');
+		$backend2
+			->expects($this->once())
+			->method('countUsers')
+			->with()
+			->willReturn(false);
+		$backend3
+			->expects($this->once())
+			->method('countUsers')
+			->with()
+			->willReturn(5);
+		$this->userManager
+			->expects($this->once())
+			->method('getBackends')
+			->with()
+			->willReturn([$backend1, $backend2, $backend3]);
 		$channels = [
 			'daily',
 			'beta',
@@ -145,6 +196,7 @@ public function testGetFormWithUpdate() {
 				'downloadLink' => 'https://downloads.nextcloud.org/server',
 				'changes' => [],
 				'webUpdaterEnabled' => true,
+				'isWebUpdaterRecommended' => true,
 				'updaterEnabled' => true,
 				'versionIsEol' => false,
 				'isDefaultUpdateServerURL' => true,
@@ -161,6 +213,42 @@ public function testGetFormWithUpdate() {
 	}
 
 	public function testGetFormWithUpdateAndChangedUpdateServer() {
+		$backend1 = $this->createMock(UserInterface::class);
+		$backend2 = $this->createMock(UserInterface::class);
+		$backend3 = $this->createMock(UserInterface::class);
+		$backend1
+			->expects($this->once())
+			->method('implementsActions')
+			->with(Backend::COUNT_USERS)
+			->willReturn(false);
+		$backend2
+			->expects($this->once())
+			->method('implementsActions')
+			->with(Backend::COUNT_USERS)
+			->willReturn(true);
+		$backend3
+			->expects($this->once())
+			->method('implementsActions')
+			->with(Backend::COUNT_USERS)
+			->willReturn(true);
+		$backend1
+			->expects($this->never())
+			->method('countUsers');
+		$backend2
+			->expects($this->once())
+			->method('countUsers')
+			->with()
+			->willReturn(false);
+		$backend3
+			->expects($this->once())
+			->method('countUsers')
+			->with()
+			->willReturn(5);
+		$this->userManager
+			->expects($this->once())
+			->method('getBackends')
+			->with()
+			->willReturn([$backend1, $backend2, $backend3]);
 		$channels = [
 			'daily',
 			'beta',
@@ -232,6 +320,7 @@ public function testGetFormWithUpdateAndChangedUpdateServer() {
 				'downloadLink' => 'https://downloads.nextcloud.org/server',
 				'changes' => [],
 				'webUpdaterEnabled' => false,
+				'isWebUpdaterRecommended' => true,
 				'updaterEnabled' => true,
 				'versionIsEol' => false,
 				'isDefaultUpdateServerURL' => false,
@@ -248,6 +337,42 @@ public function testGetFormWithUpdateAndChangedUpdateServer() {
 	}
 
 	public function testGetFormWithUpdateAndCustomersUpdateServer() {
+		$backend1 = $this->createMock(UserInterface::class);
+		$backend2 = $this->createMock(UserInterface::class);
+		$backend3 = $this->createMock(UserInterface::class);
+		$backend1
+			->expects($this->once())
+			->method('implementsActions')
+			->with(Backend::COUNT_USERS)
+			->willReturn(false);
+		$backend2
+			->expects($this->once())
+			->method('implementsActions')
+			->with(Backend::COUNT_USERS)
+			->willReturn(true);
+		$backend3
+			->expects($this->once())
+			->method('implementsActions')
+			->with(Backend::COUNT_USERS)
+			->willReturn(true);
+		$backend1
+			->expects($this->never())
+			->method('countUsers');
+		$backend2
+			->expects($this->once())
+			->method('countUsers')
+			->with()
+			->willReturn(false);
+		$backend3
+			->expects($this->once())
+			->method('countUsers')
+			->with()
+			->willReturn(5);
+		$this->userManager
+			->expects($this->once())
+			->method('getBackends')
+			->with()
+			->willReturn([$backend1, $backend2, $backend3]);
 		$channels = [
 			'daily',
 			'beta',
@@ -319,6 +444,7 @@ public function testGetFormWithUpdateAndCustomersUpdateServer() {
 				'downloadLink' => 'https://downloads.nextcloud.org/server',
 				'changes' => [],
 				'webUpdaterEnabled' => true,
+				'isWebUpdaterRecommended' => true,
 				'updaterEnabled' => true,
 				'versionIsEol' => false,
 				'isDefaultUpdateServerURL' => true,