Skip to content

Commit

Permalink
Migrate files external to PSR LoggerInterface
Browse files Browse the repository at this point in the history
Signed-off-by: Benjamin Gaussorgues <[email protected]>
  • Loading branch information
Altahrim committed Jul 19, 2023
1 parent 8b3b5ff commit 5c008be
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;

/**
* Global storages controller
Expand All @@ -49,7 +49,7 @@ class GlobalStoragesController extends StoragesController {
* @param IRequest $request request object
* @param IL10N $l10n l10n service
* @param GlobalStoragesService $globalStoragesService storage service
* @param ILogger $logger
* @param LoggerInterface $logger
* @param IUserSession $userSession
* @param IGroupManager $groupManager
* @param IConfig $config
Expand All @@ -59,7 +59,7 @@ public function __construct(
IRequest $request,
IL10N $l10n,
GlobalStoragesService $globalStoragesService,
ILogger $logger,
LoggerInterface $logger,
IUserSession $userSession,
IGroupManager $groupManager,
IConfig $config
Expand Down
57 changes: 8 additions & 49 deletions apps/files_external/lib/Controller/StoragesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,75 +42,34 @@
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;

/**
* Base class for storages controllers
*/
abstract class StoragesController extends Controller {

/**
* L10N service
*
* @var IL10N
*/
protected $l10n;

/**
* Storages service
*
* @var StoragesService
*/
protected $service;

/**
* @var ILogger
*/
protected $logger;

/**
* @var IUserSession
*/
protected $userSession;

/**
* @var IGroupManager
*/
protected $groupManager;

/**
* @var IConfig
*/
protected $config;

/**
* Creates a new storages controller.
*
* @param string $AppName application name
* @param IRequest $request request object
* @param IL10N $l10n l10n service
* @param StoragesService $storagesService storage service
* @param ILogger $logger
* @param LoggerInterface $logger
*/
public function __construct(
$AppName,
IRequest $request,
IL10N $l10n,
StoragesService $storagesService,
ILogger $logger,
IUserSession $userSession,
IGroupManager $groupManager,
IConfig $config
protected IL10N $l10n,
protected StoragesService $service,
protected LoggerInterface $logger,
protected IUserSession $userSession,
protected IGroupManager $groupManager,
protected IConfig $config
) {
parent::__construct($AppName, $request);
$this->l10n = $l10n;
$this->service = $storagesService;
$this->logger = $logger;
$this->userSession = $userSession;
$this->groupManager = $groupManager;
$this->config = $config;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;

/**
* User global storages controller
Expand All @@ -54,7 +54,7 @@ class UserGlobalStoragesController extends StoragesController {
* @param IRequest $request request object
* @param IL10N $l10n l10n service
* @param UserGlobalStoragesService $userGlobalStoragesService storage service
* @param ILogger $logger
* @param LoggerInterface $logger
* @param IUserSession $userSession
* @param IGroupManager $groupManager
*/
Expand All @@ -63,7 +63,7 @@ public function __construct(
IRequest $request,
IL10N $l10n,
UserGlobalStoragesService $userGlobalStoragesService,
ILogger $logger,
LoggerInterface $logger,
IUserSession $userSession,
IGroupManager $groupManager,
IConfig $config
Expand Down
6 changes: 3 additions & 3 deletions apps/files_external/lib/Controller/UserStoragesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;

/**
* User storages controller
Expand All @@ -53,7 +53,7 @@ class UserStoragesController extends StoragesController {
* @param IRequest $request request object
* @param IL10N $l10n l10n service
* @param UserStoragesService $userStoragesService storage service
* @param ILogger $logger
* @param LoggerInterface $logger
* @param IUserSession $userSession
* @param IGroupManager $groupManager
*/
Expand All @@ -62,7 +62,7 @@ public function __construct(
IRequest $request,
IL10N $l10n,
UserStoragesService $userStoragesService,
ILogger $logger,
LoggerInterface $logger,
IUserSession $userSession,
IGroupManager $groupManager,
IConfig $config
Expand Down
34 changes: 17 additions & 17 deletions apps/files_external/lib/Lib/Storage/SMB.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@
use Icewind\SMB\System;
use Icewind\Streams\CallbackWrapper;
use Icewind\Streams\IteratorDirectory;
use OCP\Cache\CappedMemoryCache;
use OC\Files\Filesystem;
use OC\Files\Storage\Common;
use OCA\Files_External\Lib\Notify\SMBNotifyHandler;
use OCP\Cache\CappedMemoryCache;
use OCP\Constants;
use OCP\Files\EntityTooLargeException;
use OCP\Files\Notify\IChange;
Expand All @@ -66,7 +66,7 @@
use OCP\Files\Storage\INotifyStorage;
use OCP\Files\StorageAuthException;
use OCP\Files\StorageNotAvailableException;
use OCP\ILogger;
use Psr\Log\LoggerInterface;

class SMB extends Common implements INotifyStorage {
/**
Expand All @@ -87,7 +87,7 @@ class SMB extends Common implements INotifyStorage {
/** @var CappedMemoryCache<IFileInfo> */
protected CappedMemoryCache $statCache;

/** @var ILogger */
/** @var LoggerInterface */
protected $logger;

/** @var bool */
Expand All @@ -113,7 +113,7 @@ public function __construct($params) {
if (isset($params['logger'])) {
$this->logger = $params['logger'];
} else {
$this->logger = \OC::$server->getLogger();
$this->logger = \OC::$server->get(LoggerInterface::class);
}

$options = new Options();
Expand Down Expand Up @@ -212,7 +212,7 @@ protected function getFileInfo($path) {
* @throws StorageAuthException
*/
protected function throwUnavailable(\Exception $e) {
$this->logger->logException($e, ['message' => 'Error while getting file info']);
$this->logger->error('Error while getting file info', ['exception' => $e]);
throw new StorageAuthException($e->getMessage(), $e);
}

Expand Down Expand Up @@ -277,13 +277,13 @@ protected function getFolderContents($path): iterable {
yield $file;
}
} catch (ForbiddenException $e) {
$this->logger->logException($e, ['level' => ILogger::DEBUG, 'message' => 'Hiding forbidden entry ' . $file->getName()]);
$this->logger->debug($e->getMessage(), ['exception' => $e]);
} catch (NotFoundException $e) {
$this->logger->logException($e, ['level' => ILogger::DEBUG, 'message' => 'Hiding not found entry ' . $file->getName()]);
$this->logger->debug('Hiding forbidden entry ' . $file->getName(), ['exception' => $e]);
}
}
} catch (ConnectException $e) {
$this->logger->logException($e, ['message' => 'Error while getting folder content']);
$this->logger->error('Error while getting folder content', ['exception' => $e]);
throw new StorageNotAvailableException($e->getMessage(), (int)$e->getCode(), $e);
} catch (NotFoundException $e) {
throw new \OCP\Files\NotFoundException($e->getMessage(), 0, $e);
Expand Down Expand Up @@ -328,19 +328,19 @@ public function rename($source, $target, $retry = true): bool {
$this->remove($target);
$result = $this->share->rename($absoluteSource, $absoluteTarget);
} else {
$this->logger->logException($e, ['level' => ILogger::WARN]);
$this->logger->warning($e->getMessage(), ['exception' => $e]);
return false;
}
} catch (InvalidArgumentException $e) {
if ($retry) {
$this->remove($target);
$result = $this->share->rename($absoluteSource, $absoluteTarget);
} else {
$this->logger->logException($e, ['level' => ILogger::WARN]);
$this->logger->warning($e->getMessage(), ['exception' => $e]);
return false;
}
} catch (\Exception $e) {
$this->logger->logException($e, ['level' => ILogger::WARN]);
$this->logger->warning($e->getMessage(), ['exception' => $e]);
return false;
}
unset($this->statCache[$absoluteSource], $this->statCache[$absoluteTarget]);
Expand Down Expand Up @@ -431,7 +431,7 @@ public function unlink($path) {
} catch (ForbiddenException $e) {
return false;
} catch (ConnectException $e) {
$this->logger->logException($e, ['message' => 'Error while deleting file']);
$this->logger->error('Error while deleting file', ['exception' => $e]);
throw new StorageNotAvailableException($e->getMessage(), (int)$e->getCode(), $e);
}
}
Expand Down Expand Up @@ -518,7 +518,7 @@ public function fopen($path, $mode) {
} catch (OutOfSpaceException $e) {
throw new EntityTooLargeException("not enough available space to create file", 0, $e);
} catch (ConnectException $e) {
$this->logger->logException($e, ['message' => 'Error while opening file']);
$this->logger->error('Error while opening file', ['exception' => $e]);
throw new StorageNotAvailableException($e->getMessage(), (int)$e->getCode(), $e);
}
}
Expand All @@ -545,7 +545,7 @@ public function rmdir($path) {
} catch (ForbiddenException $e) {
return false;
} catch (ConnectException $e) {
$this->logger->logException($e, ['message' => 'Error while removing folder']);
$this->logger->error('Error while removing folder', ['exception' => $e]);
throw new StorageNotAvailableException($e->getMessage(), (int)$e->getCode(), $e);
}
}
Expand All @@ -561,7 +561,7 @@ public function touch($path, $mtime = null) {
} catch (OutOfSpaceException $e) {
throw new EntityTooLargeException("not enough available space to create file", 0, $e);
} catch (ConnectException $e) {
$this->logger->logException($e, ['message' => 'Error while creating file']);
$this->logger->error('Error while creating file', ['exception' => $e]);
throw new StorageNotAvailableException($e->getMessage(), (int)$e->getCode(), $e);
}
}
Expand Down Expand Up @@ -658,7 +658,7 @@ public function mkdir($path) {
$this->share->mkdir($path);
return true;
} catch (ConnectException $e) {
$this->logger->logException($e, ['message' => 'Error while creating folder']);
$this->logger->error('Error while creating folder', ['exception' => $e]);
throw new StorageNotAvailableException($e->getMessage(), (int)$e->getCode(), $e);
} catch (Exception $e) {
return false;
Expand Down Expand Up @@ -736,7 +736,7 @@ public function test() {
} catch (ForbiddenException $e) {
return false;
} catch (Exception $e) {
$this->logger->logException($e);
$this->logger->error($e->getMessage(), ['exception' => $e]);
return false;
}
}
Expand Down
8 changes: 4 additions & 4 deletions apps/files_external/lib/Service/LegacyStoragesService.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
namespace OCA\Files_External\Service;

use OCA\Files_External\Lib\StorageConfig;
use OCP\ILogger;
use Psr\Log\LogLevel;

/**
* Read mount config from legacy mount.json
Expand Down Expand Up @@ -143,7 +143,7 @@ public function getAllStorages() {
$parts = explode('/', ltrim($rootMountPath, '/'), 3);
if (count($parts) < 3) {
// something went wrong, skip
\OC::$server->getLogger()->error('Could not parse mount point "' . $rootMountPath . '"', ['app' => 'files_external']);
OC::$server->get(LoggerInterface::class)->error('Could not parse mount point "' . $rootMountPath . '"', ['app' => 'files_external']);
continue;
}
$relativeMountPath = rtrim($parts[2], '/');
Expand Down Expand Up @@ -191,9 +191,9 @@ public function getAllStorages() {
}
} catch (\UnexpectedValueException $e) {
// don't die if a storage backend doesn't exist
\OC::$server->getLogger()->logException($e, [
OC::$server->get(LoggerInterface::class)->logException($e, [
'message' => 'Could not load storage.',
'level' => ILogger::ERROR,
'level' => LogLevel::ERROR,
'app' => 'files_external',
]);
}
Expand Down
6 changes: 3 additions & 3 deletions apps/files_external/lib/Service/StoragesService.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
use OCP\Files\Config\IUserMountCache;
use OCP\Files\Events\InvalidateMountCacheEvent;
use OCP\Files\StorageNotAvailableException;
use OCP\ILogger;
use Psr\Log\LogLevel;

/**
* Service class to manage external storage
Expand Down Expand Up @@ -121,14 +121,14 @@ protected function getStorageConfigFromDBMount(array $mount) {
// don't die if a storage backend doesn't exist
\OC::$server->getLogger()->logException($e, [
'message' => 'Could not load storage.',
'level' => ILogger::ERROR,
'level' => LogLevel::ERROR,
'app' => 'files_external',
]);
return null;
} catch (\InvalidArgumentException $e) {
\OC::$server->getLogger()->logException($e, [
'message' => 'Could not load storage.',
'level' => ILogger::ERROR,
'level' => LogLevel::ERROR,
'app' => 'files_external',
]);
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;

class GlobalStoragesControllerTest extends StoragesControllerTest {
Expand Down Expand Up @@ -65,7 +65,7 @@ private function createController($allowCreateLocal = true) {
$this->createMock(IRequest::class),
$this->createMock(IL10N::class),
$this->service,
$this->createMock(ILogger::class),
$this->createMock(LoggerInterface::class),
$session,
$this->createMock(IGroupManager::class),
$config
Expand Down
Loading

0 comments on commit 5c008be

Please sign in to comment.