Make Update notficiations strict and fix all inspections

apps/updatenotification/lib/AppInfo/Application.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2018, Joas Schilling <coding@schilljs.com>
  *

apps/updatenotification/lib/Controller/AdminController.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -76,7 +77,7 @@ public function __construct($appName,
 	 * @param string $channel
 	 * @return DataResponse
 	 */
-	public function setChannel($channel) {
+	public function setChannel(string $channel): DataResponse {
 		Util::setChannel($channel);
 		$this->config->setAppValue('core', 'lastupdatedat', 0);
 		return new DataResponse(['status' => 'success', 'data' => ['message' => $this->l10n->t('Channel updated')]]);
@@ -85,7 +86,7 @@ public function setChannel($channel) {
 	/**
 	 * @return DataResponse
 	 */
-	public function createCredentials() {
+	public function createCredentials(): DataResponse {
 		// Create a new job and store the creation date
 		$this->jobList->add(ResetTokenBackgroundJob::class);
 		$this->config->setAppValue('core', 'updater.secret.created', $this->timeFactory->getTime());

apps/updatenotification/lib/Notification/BackgroundJob.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -90,7 +91,7 @@ protected function run($argument) {
 	 * Check for ownCloud update
 	 */
 	protected function checkCoreUpdate() {
-		if (in_array($this->getChannel(), ['daily', 'git'], true)) {
+		if (\in_array($this->getChannel(), ['daily', 'git'], true)) {
 			// "These aren't the update channels you're looking for." - Ben Obi-Wan Kenobi
 			return;
 		}
@@ -102,10 +103,10 @@ protected function checkCoreUpdate() {
 			$errors = 1 + (int) $this->config->getAppValue('updatenotification', 'update_check_errors', 0);
 			$this->config->setAppValue('updatenotification', 'update_check_errors', $errors);
 
-			if (in_array($errors, $this->connectionNotifications, true)) {
+			if (\in_array($errors, $this->connectionNotifications, true)) {
 				$this->sendErrorNotifications($errors);
 			}
-		} else if (is_array($status)) {
+		} else if (\is_array($status)) {
 			$this->config->setAppValue('updatenotification', 'update_check_errors', 0);
 			$this->clearErrorNotifications();
 
@@ -178,26 +179,31 @@ protected function createNotifications($app, $version, $visibleVersion = '') {
 		if ($lastNotification === $version) {
 			// We already notified about this update
 			return;
-		} else if ($lastNotification !== false) {
+		}
+
+		if ($lastNotification !== false) {
 			// Delete old updates
 			$this->deleteOutdatedNotifications($app, $lastNotification);
 		}
 
-
 		$notification = $this->notificationManager->createNotification();
-		$notification->setApp('updatenotification')
-			->setDateTime(new \DateTime())
-			->setObject($app, $version);
-
-		if ($visibleVersion !== '') {
-			$notification->setSubject('update_available', ['version' => $visibleVersion]);
-		} else {
-			$notification->setSubject('update_available');
-		}
+		try {
+			$notification->setApp('updatenotification')
+				->setDateTime(new \DateTime())
+				->setObject($app, $version);
 
-		foreach ($this->getUsersToNotify() as $uid) {
-			$notification->setUser($uid);
-			$this->notificationManager->notify($notification);
+			if ($visibleVersion !== '') {
+				$notification->setSubject('update_available', ['version' => $visibleVersion]);
+			} else {
+				$notification->setSubject('update_available');
+			}
+
+			foreach ($this->getUsersToNotify() as $uid) {
+				$notification->setUser($uid);
+				$this->notificationManager->notify($notification);
+			}
+		} catch (\InvalidArgumentException $e) {
+			return;
 		}
 
 		$this->config->setAppValue('updatenotification', $app, $version);
@@ -206,12 +212,12 @@ protected function createNotifications($app, $version, $visibleVersion = '') {
 	/**
 	 * @return string[]
 	 */
-	protected function getUsersToNotify() {
+	protected function getUsersToNotify(): array {
 		if ($this->users !== null) {
 			return $this->users;
 		}
 
-		$notifyGroups = json_decode($this->config->getAppValue('updatenotification', 'notify_groups', '["admin"]'), true);
+		$notifyGroups = (array) json_decode($this->config->getAppValue('updatenotification', 'notify_groups', '["admin"]'), true);
 		$this->users = [];
 		foreach ($notifyGroups as $group) {
 			$groupToNotify = $this->groupManager->get($group);
@@ -235,15 +241,19 @@ protected function getUsersToNotify() {
 	 */
 	protected function deleteOutdatedNotifications($app, $version) {
 		$notification = $this->notificationManager->createNotification();
-		$notification->setApp('updatenotification')
-			->setObject($app, $version);
+		try {
+			$notification->setApp('updatenotification')
+				->setObject($app, $version);
+		} catch (\InvalidArgumentException $e) {
+			return;
+		}
 		$this->notificationManager->markProcessed($notification);
 	}
 
 	/**
 	 * @return VersionCheck
 	 */
-	protected function createVersionCheck() {
+	protected function createVersionCheck(): VersionCheck {
 		return new VersionCheck(
 			$this->client,
 			$this->config
@@ -253,7 +263,7 @@ protected function createVersionCheck() {
 	/**
 	 * @return string
 	 */
-	protected function getChannel() {
+	protected function getChannel(): string {
 		return \OC_Util::getChannel();
 	}
 

apps/updatenotification/lib/Notification/Notifier.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -33,6 +34,7 @@
 use OCP\Notification\IManager;
 use OCP\Notification\INotification;
 use OCP\Notification\INotifier;
+use OCP\Util;
 
 class Notifier implements INotifier {
 
@@ -84,17 +86,17 @@ public function __construct(IURLGenerator $url, IConfig $config, IManager $notif
 	 * @throws \InvalidArgumentException When the notification was not prepared by a notifier
 	 * @since 9.0.0
 	 */
-	public function prepare(INotification $notification, $languageCode) {
+	public function prepare(INotification $notification, $languageCode): INotification {
 		if ($notification->getApp() !== 'updatenotification') {
-			throw new \InvalidArgumentException();
+			throw new \InvalidArgumentException('Unknown app id');
 		}
 
 		$l = $this->l10NFactory->get('updatenotification', $languageCode);
 		if ($notification->getSubject() === 'connection_error') {
 			$errors = (int) $this->config->getAppValue('updatenotification', 'update_check_errors', 0);
 			if ($errors === 0) {
 				$this->notificationManager->markProcessed($notification);
-				throw new \InvalidArgumentException();
+				throw new \InvalidArgumentException('Update checked worked again');
 			}
 
 			$notification->setParsedSubject($l->t('The update server could not be reached since %d days to check for new updates.', [$errors]))
@@ -145,14 +147,14 @@ public function prepare(INotification $notification, $languageCode) {
 	protected function updateAlreadyInstalledCheck(INotification $notification, $installedVersion) {
 		if (version_compare($notification->getObjectId(), $installedVersion, '<=')) {
 			$this->notificationManager->markProcessed($notification);
-			throw new \InvalidArgumentException();
+			throw new \InvalidArgumentException('Update already installed');
 		}
 	}
 
 	/**
 	 * @return bool
 	 */
-	protected function isAdmin() {
+	protected function isAdmin(): bool {
 		$user = $this->userSession->getUser();
 
 		if ($user instanceof IUser) {
@@ -162,11 +164,11 @@ protected function isAdmin() {
 		return false;
 	}
 
-	protected function getCoreVersions() {
-		return implode('.', \OCP\Util::getVersion());
+	protected function getCoreVersions(): string {
+		return implode('.', Util::getVersion());
 	}
 
-	protected function getAppVersions() {
+	protected function getAppVersions(): array {
 		return \OC_App::getAppVersions();
 	}
 

apps/updatenotification/lib/ResetTokenBackgroundJob.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *

apps/updatenotification/lib/Settings/Admin.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -56,7 +57,7 @@ public function __construct(IConfig $config,
 	/**
 	 * @return TemplateResponse
 	 */
-	public function getForm() {
+	public function getForm(): TemplateResponse {
 		$lastUpdateCheckTimestamp = $this->config->getAppValue('core', 'lastupdatedat');
 		$lastUpdateCheck = $this->dateTimeFormatter->formatDateTime($lastUpdateCheckTimestamp);
 
@@ -99,7 +100,7 @@ public function getForm() {
 	/**
 	 * @return string the section ID, e.g. 'sharing'
 	 */
-	public function getSection() {
+	public function getSection(): string {
 		return 'server';
 	}
 
@@ -110,7 +111,7 @@ public function getSection() {
 	 *
 	 * E.g.: 70
 	 */
-	public function getPriority() {
+	public function getPriority(): int {
 		return 1;
 	}
 }

apps/updatenotification/lib/UpdateChecker.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -40,18 +41,18 @@ public function __construct(VersionCheck $updater) {
 	/**
 	 * @return array
 	 */
-	public function getUpdateState() {
+	public function getUpdateState(): array {
 		$data = $this->updater->check();
 		$result = [];
 
-		if(isset($data['version']) && $data['version'] !== '' && $data['version'] !== []) {
+		if (isset($data['version']) && $data['version'] !== '' && $data['version'] !== []) {
 			$result['updateAvailable'] = true;
 			$result['updateVersion'] = $data['versionstring'];
 			$result['updaterEnabled'] = $data['autoupdater'] === '1';
-			if(substr($data['web'], 0, 8) === 'https://') {
+			if (strpos($data['web'], 'https://') === 0) {
 				$result['updateLink'] = $data['web'];
 			}
-			if(substr($data['url'], 0, 8) === 'https://') {
+			if (strpos($data['url'], 'https://') === 0) {
 				$result['downloadLink'] = $data['url'];
 			}
 
@@ -68,7 +69,7 @@ public function populateJavaScriptVariables(array $data) {
 		$data['array']['oc_updateState'] =  json_encode([
 			'updateAvailable' => true,
 			'updateVersion' => $this->getUpdateState()['updateVersion'],
-			'updateLink' => isset($this->getUpdateState()['updateLink']) ? $this->getUpdateState()['updateLink'] : '',
+			'updateLink' => $this->getUpdateState()['updateLink'] ?? '',
 		]);
 	}
 }

apps/updatenotification/templates/admin.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 	script('updatenotification', 'admin');
 	style('updatenotification', 'admin');
 
@@ -40,7 +41,7 @@
 		<?php if (!$isDefaultUpdateServerURL) { ?>
 			<br />
 			<em>
-				<?php p($l->t("A non-default update server is in use to be checked for updates:")); ?>
+				<?php p($l->t('A non-default update server is in use to be checked for updates:')); ?>
 				<code><?php p($updateServerURL); ?></code>
 			</em>
 		<?php } ?>
@@ -65,10 +66,10 @@
 	<p id="oca_updatenotification_groups">
 		<?php p($l->t('Notify members of the following groups about available updates:')); ?>
 		<input name="oca_updatenotification_groups_list" type="hidden" id="oca_updatenotification_groups_list" value="<?php p($_['notify_groups']) ?>" style="width: 400px"><br />
-		<em class="<?php if (!in_array($currentChannel, ['daily', 'git'])) p('hidden'); ?>">
+		<em class="<?php if (!\in_array($currentChannel, ['daily', 'git'], true)) { p('hidden'); } ?>">
 			<?php p($l->t('Only notification for app updates are available.')); ?>
-			<?php if ($currentChannel === 'daily') p($l->t('The selected update channel makes dedicated notifications for the server obsolete.')); ?>
-			<?php if ($currentChannel === 'git') p($l->t('The selected update channel does not support updates of the server.')); ?>
+			<?php if ($currentChannel === 'daily') { p($l->t('The selected update channel makes dedicated notifications for the server obsolete.')); } ?>
+			<?php if ($currentChannel === 'git') { p($l->t('The selected update channel does not support updates of the server.')); } ?>
 		</em>
 	</p>
 </form>

apps/updatenotification/tests/Controller/AdminControllerTest.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -26,17 +27,13 @@
 
 use OCA\UpdateNotification\Controller\AdminController;
 use OCA\UpdateNotification\ResetTokenBackgroundJob;
-use OCA\UpdateNotification\UpdateChecker;
 use OCP\AppFramework\Http\DataResponse;
-use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\IConfig;
-use OCP\IDateTimeFormatter;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\Security\ISecureRandom;
-use OCP\Util;
 use Test\TestCase;
 
 class AdminControllerTest extends TestCase {