review address

Signed-off-by: nabim777 <[email protected]>

lib/Listener/LoadAdditionalScriptsListener.php

@@ -4,7 +4,7 @@
 
 use OCA\Files\Event\LoadAdditionalScriptsEvent;
 use OCA\OpenProject\AppInfo\Application;
-use OCA\OpenProject\VersionUtil;
+use OCA\OpenProject\ServerVersionHelper;
 use OCP\EventDispatcher\Event;
 use OCP\EventDispatcher\IEventListener;
 use OCP\Util;
@@ -13,22 +13,12 @@
  *	@template-implements IEventListener<Event>
  */
 class LoadAdditionalScriptsListener implements IEventListener {
-	/**
-	 * @var VersionUtil
-	 */
-	private $versionUtil;
-
-	public function __construct(
-		VersionUtil $versionUtil
-	) {
-		$this->versionUtil = $versionUtil;
-	}
 
 	public function handle(Event $event): void {
 		if (!$event instanceof LoadAdditionalScriptsEvent) {
 			return;
 		}
-		$nextcloudVersion = $this->versionUtil->getNextcloudVersion();
+		$nextcloudVersion = ServerVersionHelper::getNextcloudVersion();
 		if (version_compare($nextcloudVersion, '28') < 0) {
 			Util::addScript(Application::APP_ID, Application::APP_ID . '-fileActions');
 			Util::addScript(Application::APP_ID, Application::APP_ID . '-filesPluginLessThan28', 'files');

lib/ServerVersionHelper.php

@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\OpenProject;
+
+use OC_Util;
+
+class ServerVersionHelper {
+	/**
+	 * Get Nextcloud version string.
+	 *
+	 * @return string
+	 */
+	public static function getNextcloudVersion(): string {
+		// for nextcloud above 31 OC_Util::getVersionString() method does not exists
+		if (class_exists('OCP\ServerVersion')) {
+			return (new \OCP\ServerVersion())->getVersionString();
+		}
+
+		/** @psalm-suppress UndefinedMethod getVersionString() method is not in stable31 so making psalm not complain */
+		return OC_Util::getVersionString();
+	}
+}

lib/Service/OauthService.php

@@ -14,7 +14,7 @@
 use OCA\OAuth2\Db\Client;
 use OCA\OAuth2\Db\ClientMapper;
 use OCA\OAuth2\Exceptions\ClientNotFoundException;
-use OCA\OpenProject\VersionUtil;
+use OCA\OpenProject\ServerVersionHelper;
 use OCP\Security\ICrypto;
 use OCP\Security\ISecureRandom;
 
@@ -29,12 +29,6 @@ class OauthService {
 	 * @var ClientMapper
 	 */
 	private $clientMapper;
-
-	/**
-	 * @var VersionUtil
-	 */
-	private $versionUtil;
-
 	/**
 	 * @var ICrypto
 	 */
@@ -45,13 +39,11 @@ class OauthService {
 	 */
 	public function __construct(ClientMapper $clientMapper,
 		ISecureRandom $secureRandom,
-		ICrypto $crypto,
-		VersionUtil $versionUtil
+		ICrypto $crypto
 	) {
 		$this->secureRandom = $secureRandom;
 		$this->clientMapper = $clientMapper;
 		$this->crypto = $crypto;
-		$this->versionUtil = $versionUtil;
 	}
 
 	/**
@@ -88,7 +80,7 @@ public function createNcOauthClient(string $name, string $redirectUri): array {
 		$client->setName($name);
 		$client->setRedirectUri(sprintf($redirectUri, $clientId));
 		$secret = $this->secureRandom->generate(64, self::validChars);
-		$nextcloudVersion = $this->versionUtil->getNextcloudVersion();
+		$nextcloudVersion = ServerVersionHelper::getNextcloudVersion();
 		$client->setSecret($this->hashOrEncryptSecretBasedOnNextcloudVersion($secret, $nextcloudVersion));
 		$client->setClientIdentifier($clientId);
 		$client = $this->clientMapper->insert($client);

tests/acceptance/features/bootstrap/FeatureContext.php

@@ -1162,6 +1162,7 @@ public function verifyTableNodeRows(TableNode $table, array $requiredRows = [],
 	 * @param BeforeScenarioScope $scope
 	 *
 	 * @return void
+	 * @throws Exception
 	 */
 	public function before(BeforeScenarioScope $scope):void {
 		setlocale(LC_ALL, 'C.utf8');
@@ -1174,35 +1175,17 @@ public function before(BeforeScenarioScope $scope):void {
 			$this->sharingContext = $environment->getContext('SharingContext');
 			$this->directUploadContext = $environment->getContext('DirectUploadContext');
 		}
-	}
-
-	/**
-	 * @AfterScenario
-	 *
-	 * @return void
-	 * @throws Exception
-	 */
-	public function after():void {
-		foreach ($this->createdUsers as $userData) {
-			$this->theAdministratorDeletesTheUser($userData['userid']);
-		}
-		foreach ($this->createdgroups as $groups) {
-			$this->theAdministratorDeletesTheGroup($groups);
-		}
-		$this->createdAppPasswords = [];
+		$this->setGroupfolderDavPath();
 	}
 
 	/**
 	 *  This will run before EVERY scenario.
-	 *  It will set the group folder dev path
-	 *
-	 * @BeforeScenario
+	 *  It will set the group folder DAV path
 	 *
-	 * @param BeforeScenarioScope $scope
 	 * @return void
 	 * @throws Exception
 	 */
-	final public function setgroupfolderDavPath(BeforeScenarioScope $scope): void {
+	private function setGroupfolderDavPath(): void {
 		// groupfolder with version greater then 19.0.0 uses "ocs/v2.php/" endpoint
 		$capabilitiesResponse = $this->sendOCSRequest(
 			'/cloud/capabilities', 'GET', $this->getAdminUsername()
@@ -1216,4 +1199,20 @@ final public function setgroupfolderDavPath(BeforeScenarioScope $scope): void {
 			$this->groupFolderDavPath = "ocs/v2.php/";
 		}
 	}
+
+	/**
+	 * @AfterScenario
+	 *
+	 * @return void
+	 * @throws Exception
+	 */
+	public function after():void {
+		foreach ($this->createdUsers as $userData) {
+			$this->theAdministratorDeletesTheUser($userData['userid']);
+		}
+		foreach ($this->createdgroups as $groups) {
+			$this->theAdministratorDeletesTheGroup($groups);
+		}
+		$this->createdAppPasswords = [];
+	}
 }

tests/lib/Service/OauthSeviceTest.php

@@ -24,7 +24,6 @@
 namespace OCA\OpenProject\Service;
 
 use OCA\OAuth2\Db\ClientMapper;
-use OCA\OpenProject\VersionUtil;
 use OCP\Security\ICrypto;
 use OCP\Security\ISecureRandom;
 use PHPUnit\Framework\TestCase;
@@ -34,7 +33,6 @@ protected function getOauthServiceMock(
 		$clientMapperMock = null,
 		$iSecureRandomMock = null,
 		$iCryptoMock = null,
-		$versionUtil = null,
 	): OauthService {
 
 		if ($clientMapperMock === null) {
@@ -46,15 +44,10 @@ protected function getOauthServiceMock(
 		if ($iCryptoMock === null) {
 			$iCryptoMock = $this->getMockBuilder(ICrypto::class)->getMock();
 		}
-		if ($versionUtil === null) {
-			$versionUtil = $this->getMockBuilder(VersionUtil::class)->disableOriginalConstructor()->getMock();
-		}
-
 		return new OauthService(
 			$clientMapperMock,
 			$iSecureRandomMock,
 			$iCryptoMock,
-			$versionUtil
 		);
 	}