Merge pull request #44912 from nextcloud/backport/44838/stable28

2026-06-08 16:26:59 -04:00 · 2024-06-11 09:44:08 +02:00 · 2024-06-11 09:44:08 +02:00 · 436663f3c7
commit 436663f3c7
parent 974c9f2e09 4d748b9f5a
3 changed files with 74 additions and 142 deletions
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@ -75,7 +75,6 @@ use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IPreview;
 use OCP\IRequest;
-use OCP\IServerContainer;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\Lock\ILockingProvider;
@ -87,6 +86,7 @@ use OCP\Share\IManager;
 use OCP\Share\IShare;
 use OCP\UserStatus\IManager as IUserStatusManager;
 use Psr\Container\ContainerExceptionInterface;
+use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;

 /**
@ -96,32 +96,8 @@ use Psr\Log\LoggerInterface;
 */
 class ShareAPIController extends OCSController {

-	/** @var IManager */
-	private $shareManager;
-	/** @var IGroupManager */
-	private $groupManager;
-	/** @var IUserManager */
-	private $userManager;
-	/** @var IRootFolder */
-	private $rootFolder;
-	/** @var IURLGenerator */
-	private $urlGenerator;
-	/** @var string */
-	private $currentUser;
-	/** @var IL10N */
-	private $l;
-	/** @var \OCP\Files\Node */
-	private $lockedNode;
-	/** @var IConfig */
-	private $config;
-	/** @var IAppManager */
-	private $appManager;
-	/** @var IServerContainer */
-	private $serverContainer;
-	/** @var IUserStatusManager */
-	private $userStatusManager;
-	/** @var IPreview */
-	private $previewManager;
+	private ?Node $lockedNode = null;
+	private string $currentUser;

 	/**
 	 * Share20OCS constructor.
@ -129,35 +105,23 @@ class ShareAPIController extends OCSController {
 	public function __construct(
 		string $appName,
 		IRequest $request,
-		IManager $shareManager,
-		IGroupManager $groupManager,
-		IUserManager $userManager,
-		IRootFolder $rootFolder,
-		IURLGenerator $urlGenerator,
-		string $userId = null,
-		IL10N $l10n,
-		IConfig $config,
-		IAppManager $appManager,
-		IServerContainer $serverContainer,
-		IUserStatusManager $userStatusManager,
-		IPreview $previewManager,
+		private IManager $shareManager,
+		private IGroupManager $groupManager,
+		private IUserManager $userManager,
+		private IRootFolder $rootFolder,
+		private IURLGenerator $urlGenerator,
+		private IL10N $l,
+		private IConfig $config,
+		private IAppManager $appManager,
+		private ContainerInterface $serverContainer,
+		private IUserStatusManager $userStatusManager,
+		private IPreview $previewManager,
 		private IDateTimeZone $dateTimeZone,
+		private LoggerInterface $logger,
+		?string $userId = null
 	) {
 		parent::__construct($appName, $request);
-
-		$this->shareManager = $shareManager;
-		$this->userManager = $userManager;
-		$this->groupManager = $groupManager;
-		$this->request = $request;
-		$this->rootFolder = $rootFolder;
-		$this->urlGenerator = $urlGenerator;
 		$this->currentUser = $userId;
-		$this->l = $l10n;
-		$this->config = $config;
-		$this->appManager = $appManager;
-		$this->serverContainer = $serverContainer;
-		$this->userStatusManager = $userStatusManager;
-		$this->previewManager = $previewManager;
 	}

 	/**
@ -376,7 +340,7 @@ class ShareAPIController extends OCSController {
 				'strict_search' => true,
 			]);
 		} catch (Exception $e) {
-			Server::get(LoggerInterface::class)->error(
+			$this->logger->error(
 				$e->getMessage(),
 				['exception' => $e]
 			);
@ -458,7 +422,7 @@ class ShareAPIController extends OCSController {
 		try {
 			$slaveService = Server::get(\OCA\GlobalSiteSelector\Service\SlaveService::class);
 		} catch (\Throwable $e) {
-			Server::get(LoggerInterface::class)->error(
+			$this->logger->error(
 				$e->getMessage(),
 				['exception' => $e]
 			);
@ -682,6 +646,16 @@ class ShareAPIController extends OCSController {
 			$share = $this->setShareAttributes($share, $attributes);
 		}

+		//Expire date
+		if ($expireDate !== '') {
+			try {
+				$expireDateTime = $this->parseDate($expireDate);
+				$share->setExpirationDate($expireDateTime);
+			} catch (\Exception $e) {
+				throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
+			}
+		}
+
 		$share->setSharedBy($this->currentUser);
 		$this->checkInheritedAttributes($share);

@ -768,15 +742,6 @@ class ShareAPIController extends OCSController {

 			$share->setSharedWith($shareWith);
 			$share->setPermissions($permissions);
-			if ($expireDate !== '') {
-				try {
-					$expireDate = $this->parseDate($expireDate);
-					$share->setExpirationDate($expireDate);
-				} catch (\Exception $e) {
-					throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
-				}
-			}
-
 			$share->setSharedWithDisplayName($this->getCachedFederatedDisplayName($shareWith, false));
 		} elseif ($shareType === IShare::TYPE_REMOTE_GROUP) {
 			if (!$this->shareManager->outgoingServer2ServerGroupSharesAllowed()) {
@ -789,14 +754,6 @@ class ShareAPIController extends OCSController {

 			$share->setSharedWith($shareWith);
 			$share->setPermissions($permissions);
-			if ($expireDate !== '') {
-				try {
-					$expireDate = $this->parseDate($expireDate);
-					$share->setExpirationDate($expireDate);
-				} catch (\Exception $e) {
-					throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
-				}
-			}
 		} elseif ($shareType === IShare::TYPE_CIRCLE) {
 			if (!\OC::$server->getAppManager()->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) {
 				throw new OCSNotFoundException($this->l->t('You cannot share to a Circle if the app is not enabled'));
@ -832,16 +789,6 @@ class ShareAPIController extends OCSController {
 			throw new OCSBadRequestException($this->l->t('Unknown share type'));
 		}

-		//Expire date
-		if ($expireDate !== '') {
-			try {
-				$expireDate = $this->parseDate($expireDate);
-				$share->setExpirationDate($expireDate);
-			} catch (\Exception $e) {
-				throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
-			}
-		}
-
 		$share->setShareType($shareType);

 		if ($note !== '') {
--- a/apps/files_sharing/tests/ApiTest.php
+++ b/apps/files_sharing/tests/ApiTest.php
@ -48,9 +48,10 @@ use OCP\IDateTimeZone;
 use OCP\IL10N;
 use OCP\IPreview;
 use OCP\IRequest;
-use OCP\IServerContainer;
 use OCP\Share\IShare;
 use OCP\UserStatus\IManager as IUserStatusManager;
+use Psr\Container\ContainerInterface;
+use Psr\Log\LoggerInterface;

 /**
 * Class ApiTest
@ -120,10 +121,11 @@ class ApiTest extends TestCase {
 			});
 		$config = $this->createMock(IConfig::class);
 		$appManager = $this->createMock(IAppManager::class);
-		$serverContainer = $this->createMock(IServerContainer::class);
+		$serverContainer = $this->createMock(ContainerInterface::class);
 		$userStatusManager = $this->createMock(IUserStatusManager::class);
 		$previewManager = $this->createMock(IPreview::class);
 		$dateTimeZone = $this->createMock(IDateTimeZone::class);
+		$logger = $this->createMock(LoggerInterface::class);
 		$dateTimeZone->method('getTimeZone')->willReturn(new \DateTimeZone(date_default_timezone_get()));

 		return new ShareAPIController(
@ -134,7 +136,6 @@ class ApiTest extends TestCase {
 			\OC::$server->getUserManager(),
 			\OC::$server->getRootFolder(),
 			\OC::$server->getURLGenerator(),
-			$userId,
 			$l,
 			$config,
 			$appManager,
@ -142,6 +143,8 @@ class ApiTest extends TestCase {
 			$userStatusManager,
 			$previewManager,
 			$dateTimeZone,
+			$logger,
+			$userId,
 		);
 	}

--- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
@ -53,7 +53,6 @@ use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IPreview;
 use OCP\IRequest;
-use OCP\IServerContainer;
 use OCP\IURLGenerator;
 use OCP\IUser;
 use OCP\IUserManager;
@ -63,6 +62,8 @@ use OCP\Share\IAttributes as IShareAttributes;
 use OCP\Share\IManager;
 use OCP\Share\IShare;
 use OCP\UserStatus\IManager as IUserStatusManager;
+use Psr\Container\ContainerInterface;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;

 /**
@ -73,53 +74,23 @@ use Test\TestCase;
 */
 class ShareAPIControllerTest extends TestCase {

-	/** @var string */
-	private $appName = 'files_sharing';
-
-	/** @var \OC\Share20\Manager|\PHPUnit\Framework\MockObject\MockObject */
-	private $shareManager;
-
-	/** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */
-	private $groupManager;
-
-	/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
-	private $userManager;
-
-	/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */
-	private $request;
-
-	/** @var IRootFolder|\PHPUnit\Framework\MockObject\MockObject */
-	private $rootFolder;
-
-	/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */
-	private $urlGenerator;
-
-	/** @var string|\PHPUnit\Framework\MockObject\MockObject */
-	private $currentUser;
-
-	/** @var ShareAPIController */
-	private $ocs;
-
-	/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
-	private $l;
-
-	/** @var  IConfig|\PHPUnit\Framework\MockObject\MockObject */
-	private $config;
-
-	/** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject */
-	private $appManager;
-
-	/** @var IServerContainer|\PHPUnit\Framework\MockObject\MockObject */
-	private $serverContainer;
-
-	/** @var IUserStatusManager|\PHPUnit\Framework\MockObject\MockObject */
-	private $userStatusManager;
-
-	/** @var IPreview|\PHPUnit\Framework\MockObject\MockObject */
-	private $previewManager;
-
-	/** @var IDateTimeZone|\PHPUnit\Framework\MockObject\MockObject */
-	private $dateTimeZone;
+	private string $appName = 'files_sharing';
+	private \OC\Share20\Manager|\PHPUnit\Framework\MockObject\MockObject $shareManager;
+	private IGroupManager|\PHPUnit\Framework\MockObject\MockObject $groupManager;
+	private IUserManager|\PHPUnit\Framework\MockObject\MockObject $userManager;
+	private IRequest|\PHPUnit\Framework\MockObject\MockObject $request;
+	private IRootFolder|\PHPUnit\Framework\MockObject\MockObject $rootFolder;
+	private IURLGenerator|\PHPUnit\Framework\MockObject\MockObject $urlGenerator;
+	private string|\PHPUnit\Framework\MockObject\MockObject $currentUser;
+	private ShareAPIController  $ocs;
+	private IL10N|\PHPUnit\Framework\MockObject\MockObject $l;
+	private IConfig|\PHPUnit\Framework\MockObject\MockObject $config;
+	private IAppManager|\PHPUnit\Framework\MockObject\MockObject $appManager;
+	private IServerContainer|\PHPUnit\Framework\MockObject\MockObject $serverContainer;
+	private IUserStatusManager|\PHPUnit\Framework\MockObject\MockObject $userStatusManager;
+	private IPreview|\PHPUnit\Framework\MockObject\MockObject $previewManager;
+	private IDateTimeZone|\PHPUnit\Framework\MockObject\MockObject $dateTimeZone;
+	private LoggerInterface $logger;

 	protected function setUp(): void {
 		$this->shareManager = $this->createMock(IManager::class);
@ -144,7 +115,7 @@ class ShareAPIControllerTest extends TestCase {
 			});
 		$this->config = $this->createMock(IConfig::class);
 		$this->appManager = $this->createMock(IAppManager::class);
-		$this->serverContainer = $this->createMock(IServerContainer::class);
+		$this->serverContainer = $this->createMock(ContainerInterface::class);
 		$this->userStatusManager = $this->createMock(IUserStatusManager::class);
 		$this->previewManager = $this->createMock(IPreview::class);
 		$this->previewManager->method('isAvailable')
@ -152,6 +123,7 @@ class ShareAPIControllerTest extends TestCase {
 				return $fileInfo->getMimeType() === 'mimeWithPreview';
 			});
 		$this->dateTimeZone = $this->createMock(IDateTimeZone::class);
+		$this->logger = $this->createMock(LoggerInterface::class);

 		$this->ocs = new ShareAPIController(
 			$this->appName,
@ -161,7 +133,6 @@ class ShareAPIControllerTest extends TestCase {
 			$this->userManager,
 			$this->rootFolder,
 			$this->urlGenerator,
-			$this->currentUser,
 			$this->l,
 			$this->config,
 			$this->appManager,
@ -169,6 +140,8 @@ class ShareAPIControllerTest extends TestCase {
 			$this->userStatusManager,
 			$this->previewManager,
 			$this->dateTimeZone,
+			$this->logger,
+			$this->currentUser,
 		);
 	}

@ -185,7 +158,6 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userManager,
 				$this->rootFolder,
 				$this->urlGenerator,
-				$this->currentUser,
 				$this->l,
 				$this->config,
 				$this->appManager,
@ -193,6 +165,8 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userStatusManager,
 				$this->previewManager,
 				$this->dateTimeZone,
+				$this->logger,
+				$this->currentUser,
 			])->setMethods(['formatShare'])
 			->getMock();
 	}
@ -783,7 +757,6 @@ class ShareAPIControllerTest extends TestCase {
 					$this->userManager,
 					$this->rootFolder,
 					$this->urlGenerator,
-					$this->currentUser,
 					$this->l,
 					$this->config,
 					$this->appManager,
@ -791,6 +764,9 @@ class ShareAPIControllerTest extends TestCase {
 					$this->userStatusManager,
 					$this->previewManager,
 					$this->dateTimeZone,
+					$this->logger,
+					$this->currentUser,
+
 				])->setMethods(['canAccessShare'])
 				->getMock();

@ -1409,7 +1385,6 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userManager,
 				$this->rootFolder,
 				$this->urlGenerator,
-				$this->currentUser,
 				$this->l,
 				$this->config,
 				$this->appManager,
@ -1417,6 +1392,8 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userStatusManager,
 				$this->previewManager,
 				$this->dateTimeZone,
+				$this->logger,
+				$this->currentUser,
 			])->setMethods(['formatShare'])
 			->getMock();

@ -1749,7 +1726,6 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userManager,
 				$this->rootFolder,
 				$this->urlGenerator,
-				$this->currentUser,
 				$this->l,
 				$this->config,
 				$this->appManager,
@ -1757,6 +1733,8 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userStatusManager,
 				$this->previewManager,
 				$this->dateTimeZone,
+				$this->logger,
+				$this->currentUser,
 			])->setMethods(['formatShare'])
 			->getMock();

@ -1844,7 +1822,6 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userManager,
 				$this->rootFolder,
 				$this->urlGenerator,
-				$this->currentUser,
 				$this->l,
 				$this->config,
 				$this->appManager,
@ -1852,6 +1829,8 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userStatusManager,
 				$this->previewManager,
 				$this->dateTimeZone,
+				$this->logger,
+				$this->currentUser,
 			])->setMethods(['formatShare'])
 			->getMock();

@ -2254,7 +2233,6 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userManager,
 				$this->rootFolder,
 				$this->urlGenerator,
-				$this->currentUser,
 				$this->l,
 				$this->config,
 				$this->appManager,
@ -2262,6 +2240,8 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userStatusManager,
 				$this->previewManager,
 				$this->dateTimeZone,
+				$this->logger,
+				$this->currentUser,
 			])->setMethods(['formatShare'])
 			->getMock();

@ -2321,7 +2301,6 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userManager,
 				$this->rootFolder,
 				$this->urlGenerator,
-				$this->currentUser,
 				$this->l,
 				$this->config,
 				$this->appManager,
@ -2329,6 +2308,8 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userStatusManager,
 				$this->previewManager,
 				$this->dateTimeZone,
+				$this->logger,
+				$this->currentUser,
 			])->setMethods(['formatShare'])
 			->getMock();

@ -2561,7 +2542,6 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userManager,
 				$this->rootFolder,
 				$this->urlGenerator,
-				$this->currentUser,
 				$this->l,
 				$this->config,
 				$this->appManager,
@ -2569,6 +2549,8 @@ class ShareAPIControllerTest extends TestCase {
 				$this->userStatusManager,
 				$this->previewManager,
 				$this->dateTimeZone,
+				$this->logger,
+				$this->currentUser,
 			])->setMethods(['formatShare'])
 			->getMock();