Skip to content

Commit

Permalink
refactor(shareApiController): use contrusctor property promotion & DI…
Browse files Browse the repository at this point in the history 
… logger

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
  • Loading branch information
@Fenn-CS
Fenn-CS committed Apr 18, 2024
1 parent 2a9a5b3 commit abd24c8
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 103 deletions.
49 changes: 7 additions & 42 deletions apps/files_sharing/lib/Controller/ShareAPIController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@
use OCP\IL10N;
use OCP\IPreview;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\IURLGenerator;
use OCP\IUserManager;
use OCP\Lock\ILockingProvider;
Expand All @@ -87,6 +86,7 @@
use OCP\Share\IShare;
use OCP\UserStatus\IManager as IUserStatusManager;
use Psr\Container\ContainerExceptionInterface;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;

/**
Expand All @@ -96,32 +96,8 @@
*/
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.
Expand All @@ -142,22 +118,11 @@ public function __construct(
IUserStatusManager $userStatusManager,
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;
}

/**
Expand Down Expand Up @@ -376,7 +341,7 @@ private function getDisplayNameFromAddressBook(string $query, string $property):
'strict_search' => true,
]);
} catch (Exception $e) {
Server::get(LoggerInterface::class)->error(
$this->logger->error(
$e->getMessage(),
['exception' => $e]
);
Expand Down Expand Up @@ -458,7 +423,7 @@ private function retrieveFederatedDisplayName(array $userIds, bool $cacheOnly =
try {
$slaveService = Server::get(\OCA\GlobalSiteSelector\Service\SlaveService::class);
} catch (\Throwable $e) {
Server::get(LoggerInterface::class)->error(
$this->logger->error(
$e->getMessage(),
['exception' => $e]
);
Expand Down
9 changes: 6 additions & 3 deletions apps/files_sharing/tests/ApiTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@
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
Expand Down Expand Up @@ -120,10 +121,11 @@ private function createOCS($userId) {
});
$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(
Expand All @@ -134,14 +136,15 @@ private function createOCS($userId) {
\OC::$server->getUserManager(),
\OC::$server->getRootFolder(),
\OC::$server->getURLGenerator(),
$userId,
$l,
$config,
$appManager,
$serverContainer,
$userStatusManager,
$previewManager,
$dateTimeZone,
$logger,
$userId,
);
}

Expand Down
98 changes: 40 additions & 58 deletions apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
use OCP\IL10N;
use OCP\IPreview;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
Expand All @@ -63,6 +62,8 @@
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;

/**
Expand All @@ -73,53 +74,23 @@
*/
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);
Expand All @@ -144,14 +115,15 @@ protected function setUp(): void {
});
$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')
->willReturnCallback(function ($fileInfo) {
return $fileInfo->getMimeType() === 'mimeWithPreview';
});
$this->dateTimeZone = $this->createMock(IDateTimeZone::class);
$this->logger = $this->createMock(LoggerInterface::class);

$this->ocs = new ShareAPIController(
$this->appName,
Expand All @@ -161,14 +133,15 @@ protected function setUp(): void {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
);
}

Expand All @@ -185,14 +158,15 @@ private function mockFormatShare() {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();
}
Expand Down Expand Up @@ -783,14 +757,16 @@ public function testGetShare(\OCP\Share\IShare $share, array $result) {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,

])->setMethods(['canAccessShare'])
->getMock();

Expand Down Expand Up @@ -1409,14 +1385,15 @@ public function testGetShares(array $getSharesParameters, array $shares, array $
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -1749,14 +1726,15 @@ public function testCreateShareUser() {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -1844,14 +1822,15 @@ public function testCreateShareGroup() {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -2254,14 +2233,15 @@ public function testCreateShareRemote() {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -2321,14 +2301,15 @@ public function testCreateShareRemoteGroup() {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -2561,14 +2542,15 @@ public function testCreateReshareOfFederatedMountNoDeletePermissions() {
$this->userManager,
$this->rootFolder,
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config,
$this->appManager,
$this->serverContainer,
$this->userStatusManager,
$this->previewManager,
$this->dateTimeZone,
$this->logger,
$this->currentUser,
])->setMethods(['formatShare'])
->getMock();

Expand Down

0 comments on commit abd24c8

Please sign in to comment.