Skip to content

Move away from deprecated ILogger #31752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
come-nc merged 7 commits into from
May 2, 2022
Merged

Move away from deprecated ILogger #31752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
51f04ed
Move away from deprecated ILogger
come-nc Mar 29, 2022
0654b24
Fix identation
come-nc Mar 30, 2022
069477e
Migrate more classes of lib/private to LoggerInterface
come-nc Mar 30, 2022
23f7a37
Remove ILogger usages in lib/private/Files/Storage
come-nc Mar 31, 2022
a795ad4
Remove unused class SearchResultSorter
come-nc Mar 31, 2022
2b07031
composer run cs:fix
come-nc Apr 4, 2022
8184e35
Remove test for removed class SearchResultSorter
come-nc Apr 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion lib/composer/composer/autoload_classmap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1501,7 +1501,6 @@
'OC\\Share20\\UserRemovedListener' => $baseDir . '/lib/private/Share20/UserRemovedListener.php',
'OC\\Share\\Constants' => $baseDir . '/lib/private/Share/Constants.php',
'OC\\Share\\Helper' => $baseDir . '/lib/private/Share/Helper.php',
'OC\\Share\\SearchResultSorter' => $baseDir . '/lib/private/Share/SearchResultSorter.php',
'OC\\Share\\Share' => $baseDir . '/lib/private/Share/Share.php',
'OC\\StreamImage' => $baseDir . '/lib/private/StreamImage.php',
'OC\\Streamer' => $baseDir . '/lib/private/Streamer.php',
Expand Down
1 change: 0 additions & 1 deletion lib/composer/composer/autoload_static.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1530,7 +1530,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\Share20\\UserRemovedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/UserRemovedListener.php',
'OC\\Share\\Constants' => __DIR__ . '/../../..' . '/lib/private/Share/Constants.php',
'OC\\Share\\Helper' => __DIR__ . '/../../..' . '/lib/private/Share/Helper.php',
'OC\\Share\\SearchResultSorter' => __DIR__ . '/../../..' . '/lib/private/Share/SearchResultSorter.php',
'OC\\Share\\Share' => __DIR__ . '/../../..' . '/lib/private/Share/Share.php',
'OC\\StreamImage' => __DIR__ . '/../../..' . '/lib/private/StreamImage.php',
'OC\\Streamer' => __DIR__ . '/../../..' . '/lib/private/Streamer.php',
Expand Down
4 changes: 2 additions & 2 deletions lib/private/Cache/File.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
use OC\Files\Filesystem;
use OC\Files\View;
use OCP\ICache;
use OCP\ILogger;
use OCP\Security\ISecureRandom;
use Psr\Log\LoggerInterface;

class File implements ICache {

Expand Down Expand Up @@ -61,7 +61,7 @@ protected function getStorage() {
$this->storage = new View('/' . $user->getUID() . '/cache');
return $this->storage;
} else {
\OCP\Util::writeLog('core', 'Can\'t get cache storage, user not logged in', ILogger::ERROR);
\OC::$server->get(LoggerInterface::class)->error('Can\'t get cache storage, user not logged in', ['app' => 'core']);
throw new \OC\ForbiddenException('Can\t get cache storage, user not logged in');
}
}
Expand Down
37 changes: 20 additions & 17 deletions lib/private/Dashboard/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
use OCP\AppFramework\QueryException;
use OCP\Dashboard\IManager;
use OCP\Dashboard\IWidget;
use OCP\ILogger;
use OCP\IServerContainer;
use Throwable;
use Psr\Log\LoggerInterface;

class Manager implements IManager {

Expand Down Expand Up @@ -72,10 +72,10 @@ public function loadLazyPanels(): void {
* There is a circular dependency between the logger and the registry, so
* we can not inject it. Thus the static call.
*/
\OC::$server->getLogger()->logException($e, [
'message' => 'Could not load lazy dashbaord widget: ' . $e->getMessage(),
'level' => ILogger::FATAL,
]);
\OC::$server->get(LoggerInterface::class)->critical(
'Could not load lazy dashboard widget: ' . $e->getMessage(),
['excepiton' => $e]
);
}
/**
* Try to register the loaded reporter. Theoretically it could be of a wrong
Expand All @@ -88,10 +88,10 @@ public function loadLazyPanels(): void {
* There is a circular dependency between the logger and the registry, so
* we can not inject it. Thus the static call.
*/
\OC::$server->getLogger()->logException($e, [
'message' => 'Could not register lazy dashboard widget: ' . $e->getMessage(),
'level' => ILogger::FATAL,
]);
\OC::$server->get(LoggerInterface::class)->critical(
'Could not register lazy dashboard widget: ' . $e->getMessage(),
['excepiton' => $e]
);
}

try {
Expand All @@ -100,16 +100,19 @@ public function loadLazyPanels(): void {
$endTime = microtime(true);
$duration = $endTime - $startTime;
if ($duration > 1) {
\OC::$server->getLogger()->error('Dashboard widget {widget} took {duration} seconds to load.', [
'widget' => $widget->getId(),
'duration' => round($duration, 2),
]);
\OC::$server->get(LoggerInterface::class)->error(
'Dashboard widget {widget} took {duration} seconds to load.',
[
'widget' => $widget->getId(),
'duration' => round($duration, 2),
]
);
}
} catch (Throwable $e) {
\OC::$server->getLogger()->logException($e, [
'message' => 'Error during dashboard widget loading: ' . $e->getMessage(),
'level' => ILogger::FATAL,
]);
\OC::$server->get(LoggerInterface::class)->critical(
'Error during dashboard widget loading: ' . $e->getMessage(),
['excepiton' => $e]
);
}
}
$this->lazyWidgets = [];
Expand Down
6 changes: 3 additions & 3 deletions lib/private/DateTimeZone.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

use OCP\IConfig;
use OCP\IDateTimeZone;
use OCP\ILogger;
use OCP\ISession;
use Psr\Log\LoggerInterface;

class DateTimeZone implements IDateTimeZone {
/** @var IConfig */
Expand Down Expand Up @@ -65,7 +65,7 @@ public function getTimeZone($timestamp = false) {
try {
return new \DateTimeZone($timeZone);
} catch (\Exception $e) {
\OCP\Util::writeLog('datetimezone', 'Failed to created DateTimeZone "' . $timeZone . "'", ILogger::DEBUG);
\OC::$server->get(LoggerInterface::class)->debug('Failed to created DateTimeZone "' . $timeZone . '"', ['app' => 'datetimezone']);
return new \DateTimeZone($this->getDefaultTimeZone());
}
}
Expand Down Expand Up @@ -110,7 +110,7 @@ protected function guessTimeZoneFromOffset($offset, $timestamp) {
}

// No timezone found, fallback to UTC
\OCP\Util::writeLog('datetimezone', 'Failed to find DateTimeZone for offset "' . $offset . "'", ILogger::DEBUG);
\OC::$server->get(LoggerInterface::class)->debug('Failed to find DateTimeZone for offset "' . $offset . '"', ['app' => 'datetimezone']);
return new \DateTimeZone($this->getDefaultTimeZone());
}
}
Expand Down
17 changes: 8 additions & 9 deletions lib/private/Files/Cache/Scanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@
namespace OC\Files\Cache;

use Doctrine\DBAL\Exception;
use OC\Files\Storage\Wrapper\Jail;
use OC\Files\Storage\Wrapper\Encoding;
use OC\Hooks\BasicEmitter;
use OCP\Files\Cache\IScanner;
use OCP\Files\ForbiddenException;
use OCP\Files\Storage\IReliableEtagStorage;
use OCP\ILogger;
use OCP\Lock\ILockingProvider;
use OC\Files\Storage\Wrapper\Encoding;
use OC\Files\Storage\Wrapper\Jail;
use OC\Hooks\BasicEmitter;
use Psr\Log\LoggerInterface;

/**
* Class Scanner
Expand Down Expand Up @@ -115,7 +115,7 @@ public function setUseTransactions($useTransactions) {
protected function getData($path) {
$data = $this->storage->getMetaData($path);
if (is_null($data)) {
\OCP\Util::writeLog(Scanner::class, "!!! Path '$path' is not accessible or present !!!", ILogger::DEBUG);
\OC::$server->get(LoggerInterface::class)->debug("!!! Path '$path' is not accessible or present !!!", ['app' => 'core']);
}
return $data;
}
Expand Down Expand Up @@ -425,7 +425,7 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
$file = trim(\OC\Files\Filesystem::normalizePath($originalFile), '/');
if (trim($originalFile, '/') !== $file) {
// encoding mismatch, might require compatibility wrapper
\OC::$server->getLogger()->debug('Scanner: Skipping non-normalized file name "'. $originalFile . '" in path "' . $path . '".', ['app' => 'core']);
\OC::$server->get(LoggerInterface::class)->debug('Scanner: Skipping non-normalized file name "'. $originalFile . '" in path "' . $path . '".', ['app' => 'core']);
$this->emit('\OC\Files\Cache\Scanner', 'normalizedNameMismatch', [$path ? $path . '/' . $originalFile : $originalFile]);
// skip this entry
continue;
Expand Down Expand Up @@ -456,10 +456,9 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
\OC::$server->getDatabaseConnection()->rollback();
\OC::$server->getDatabaseConnection()->beginTransaction();
}
\OC::$server->getLogger()->logException($ex, [
'message' => 'Exception while scanning file "' . $child . '"',
'level' => ILogger::DEBUG,
\OC::$server->get(LoggerInterface::class)->debug('Exception while scanning file "' . $child . '"', [
'app' => 'core',
'exception' => $ex,
]);
$exceptionOccurred = true;
} catch (\OCP\Lock\LockedException $e) {
Expand Down
6 changes: 3 additions & 3 deletions lib/private/Files/Mount/MountPoint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
use OC\Files\Storage\StorageFactory;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\Storage\IStorageFactory;
use OCP\ILogger;
use Psr\Log\LoggerInterface;

class MountPoint implements IMountPoint {
/**
Expand Down Expand Up @@ -173,12 +173,12 @@ private function createStorage() {
// the root storage could not be initialized, show the user!
throw new \Exception('The root storage could not be initialized. Please contact your local administrator.', $exception->getCode(), $exception);
} else {
\OC::$server->getLogger()->logException($exception, ['level' => ILogger::ERROR]);
\OC::$server->get(LoggerInterface::class)->error($exception->getMessage(), ['exception' => $exception]);
}
return;
}
} else {
\OCP\Util::writeLog('core', 'storage backend ' . $this->class . ' not found', ILogger::ERROR);
\OC::$server->get(LoggerInterface::class)->error('Storage backend ' . $this->class . ' not found', ['app' => 'core']);
$this->invalidStorage = true;
return;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/private/Files/Mount/ObjectHomeMountProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
use OCP\Files\Config\IHomeMountProvider;
use OCP\Files\Storage\IStorageFactory;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUser;
use Psr\Log\LoggerInterface;

/**
* Mount provider for object store home storages
Expand Down Expand Up @@ -80,7 +80,7 @@ private function getSingleBucketObjectStoreConfig(IUser $user) {

// sanity checks
if (empty($config['class'])) {
\OCP\Util::writeLog('files', 'No class given for objectstore', ILogger::ERROR);
\OC::$server->get(LoggerInterface::class)->error('No class given for objectstore', ['app' => 'files']);
}
if (!isset($config['arguments'])) {
$config['arguments'] = [];
Expand All @@ -105,7 +105,7 @@ private function getMultiBucketObjectStoreConfig(IUser $user) {

// sanity checks
if (empty($config['class'])) {
\OCP\Util::writeLog('files', 'No class given for objectstore', ILogger::ERROR);
\OC::$server->get(LoggerInterface::class)->error('No class given for objectstore', ['app' => 'files']);
}
if (!isset($config['arguments'])) {
$config['arguments'] = [];
Expand Down
11 changes: 5 additions & 6 deletions lib/private/Files/ObjectStore/S3ConnectionTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
use GuzzleHttp\Promise;
use GuzzleHttp\Promise\RejectedPromise;
use OCP\ICertificateManager;
use OCP\ILogger;
use Psr\Log\LoggerInterface;

trait S3ConnectionTrait {
/** @var array */
Expand Down Expand Up @@ -150,13 +150,13 @@ public function getConnection() {
$this->connection = new S3Client($options);

if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
$logger = \OC::$server->getLogger();
$logger = \OC::$server->get(LoggerInterface::class);
$logger->debug('Bucket "' . $this->bucket . '" This bucket name is not dns compatible, it may contain invalid characters.',
['app' => 'objectstore']);
}

if ($this->params['verify_bucket_exists'] && !$this->connection->doesBucketExist($this->bucket)) {
$logger = \OC::$server->getLogger();
$logger = \OC::$server->get(LoggerInterface::class);
try {
$logger->info('Bucket "' . $this->bucket . '" does not exist - creating it.', ['app' => 'objectstore']);
if (!$this->connection::isBucketDnsCompatible($this->bucket)) {
Expand All @@ -165,9 +165,8 @@ public function getConnection() {
$this->connection->createBucket(['Bucket' => $this->bucket]);
$this->testTimeout();
} catch (S3Exception $e) {
$logger->logException($e, [
'message' => 'Invalid remote storage.',
'level' => ILogger::DEBUG,
$logger->debug('Invalid remote storage.', [
'exception' => $e,
'app' => 'objectstore',
]);
throw new \Exception('Creation of bucket "' . $this->bucket . '" failed. ' . $e->getMessage());
Expand Down
28 changes: 17 additions & 11 deletions lib/private/Files/Storage/Common.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@
use OCP\Files\Storage\ILockingStorage;
use OCP\Files\Storage\IStorage;
use OCP\Files\Storage\IWriteStreamStorage;
use OCP\ILogger;
use OCP\Lock\ILockingProvider;
use OCP\Lock\LockedException;
use Psr\Log\LoggerInterface;

/**
* Storage backend class for providing common filesystem operation methods
Expand All @@ -89,7 +89,9 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
protected $mountOptions = [];
protected $owner = null;

/** @var ?bool */
private $shouldLogLocks = null;
/** @var ?LoggerInterface */
private $logger;

public function __construct($parameters) {
Expand Down Expand Up @@ -237,7 +239,7 @@ public function copy($path1, $path2) {
$target = $this->fopen($path2, 'w');
[, $result] = \OC_Helper::streamCopy($source, $target);
if (!$result) {
\OC::$server->getLogger()->warning("Failed to write data while copying $path1 to $path2");
\OC::$server->get(LoggerInterface::class)->warning("Failed to write data while copying $path1 to $path2");
}
$this->removeCachedFile($path2);
return $result;
Expand Down Expand Up @@ -459,11 +461,13 @@ public function test() {
if ($this->stat('')) {
return true;
}
\OC::$server->getLogger()->info("External storage not available: stat() failed");
\OC::$server->get(LoggerInterface::class)->info("External storage not available: stat() failed");
return false;
} catch (\Exception $e) {
\OC::$server->getLogger()->warning("External storage not available: " . $e->getMessage());
\OC::$server->getLogger()->logException($e, ['level' => ILogger::WARN]);
\OC::$server->get(LoggerInterface::class)->warning(
"External storage not available: " . $e->getMessage(),
['exception' => $e]
);
return false;
}
}
Expand Down Expand Up @@ -628,7 +632,7 @@ public function copyFromStorage(IStorage $sourceStorage, $sourceInternalPath, $t
$this->writeStream($targetInternalPath, $source);
$result = true;
} catch (\Exception $e) {
\OC::$server->getLogger()->logException($e, ['level' => ILogger::WARN, 'message' => 'Failed to copy stream to storage']);
\OC::$server->get(LoggerInterface::class)->warning('Failed to copy stream to storage', ['exception' => $e]);
}
}

Expand Down Expand Up @@ -758,7 +762,7 @@ public function acquireLock($path, $type, ILockingProvider $provider) {
$provider->acquireLock('files/' . md5($this->getId() . '::' . trim($path, '/')), $type, $this->getId() . '::' . $path);
} catch (LockedException $e) {
if ($logger) {
$logger->logException($e, ['level' => ILogger::INFO]);
$logger->info($e->getMessage(), ['exception' => $e]);
}
throw $e;
}
Expand Down Expand Up @@ -790,7 +794,7 @@ public function releaseLock($path, $type, ILockingProvider $provider) {
$provider->releaseLock('files/' . md5($this->getId() . '::' . trim($path, '/')), $type);
} catch (LockedException $e) {
if ($logger) {
$logger->logException($e, ['level' => ILogger::INFO]);
$logger->info($e->getMessage(), ['exception' => $e]);
}
throw $e;
}
Expand Down Expand Up @@ -821,15 +825,17 @@ public function changeLock($path, $type, ILockingProvider $provider) {
try {
$provider->changeLock('files/' . md5($this->getId() . '::' . trim($path, '/')), $type);
} catch (LockedException $e) {
\OC::$server->getLogger()->logException($e, ['level' => ILogger::INFO]);
if ($logger) {
$logger->info($e->getMessage(), ['exception' => $e]);
}
throw $e;
}
}

private function getLockLogger() {
private function getLockLogger(): ?LoggerInterface {
if (is_null($this->shouldLogLocks)) {
$this->shouldLogLocks = \OC::$server->getConfig()->getSystemValue('filelocking.debug', false);
$this->logger = $this->shouldLogLocks ? \OC::$server->getLogger() : null;
$this->logger = $this->shouldLogLocks ? \OC::$server->get(LoggerInterface::class) : null;
}
return $this->logger;
}
Expand Down
7 changes: 3 additions & 4 deletions lib/private/Files/Storage/DAV.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,12 @@
use OCP\Files\StorageNotAvailableException;
use OCP\Http\Client\IClientService;
use OCP\ICertificateManager;
use OCP\ILogger;
use OCP\Util;
use Psr\Http\Message\ResponseInterface;
use Sabre\DAV\Client;
use Sabre\DAV\Xml\Property\ResourceType;
use Sabre\HTTP\ClientException;
use Sabre\HTTP\ClientHttpException;
use Psr\Log\LoggerInterface;

/**
* Class DAV
Expand Down Expand Up @@ -370,7 +369,7 @@ public function fopen($path, $mode) {
if ($response->getStatusCode() === Http::STATUS_LOCKED) {
throw new \OCP\Lock\LockedException($path);
} else {
Util::writeLog("webdav client", 'Guzzle get returned status code ' . $response->getStatusCode(), ILogger::ERROR);
\OC::$server->get(LoggerInterface::class)->error('Guzzle get returned status code ' . $response->getStatusCode(), ['app' => 'webdav client']);
}
}

Expand Down Expand Up @@ -843,7 +842,7 @@ public function hasUpdated($path, $time) {
* @throws ForbiddenException if the action is not allowed
*/
protected function convertException(Exception $e, $path = '') {
\OC::$server->getLogger()->logException($e, ['app' => 'files_external', 'level' => ILogger::DEBUG]);
\OC::$server->get(LoggerInterface::class)->debug($e->getMessage(), ['app' => 'files_external', 'exception' => $e]);
if ($e instanceof ClientHttpException) {
if ($e->getHttpStatus() === Http::STATUS_LOCKED) {
throw new \OCP\Lock\LockedException($path);
Expand Down
Loading