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 1 commit
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
Prev Previous commit
Next Next commit
Remove ILogger usages in lib/private/Files/Storage 
Signed-off-by: Côme Chilliet <[email protected]>
  • Loading branch information
@come-nc
come-nc committed Apr 26, 2022
commit 23f7a37c03551b42f751b65ebeceeb1838b36039
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
6 changes: 3 additions & 3 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,13 @@
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 +370,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 +843,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
10 changes: 5 additions & 5 deletions lib/private/Files/Storage/Local.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
use OCP\Files\IMimeTypeDetector;
use OCP\Files\Storage\IStorage;
use OCP\IConfig;
use OCP\ILogger;
use Psr\Log\LoggerInterface;

/**
* for local filestore, we only have to map the paths
Expand Down Expand Up @@ -323,17 +323,17 @@ public function rename($path1, $path2) {
$dstParent = dirname($path2);

if (!$this->isUpdatable($srcParent)) {
\OCP\Util::writeLog('core', 'unable to rename, source directory is not writable : ' . $srcParent, ILogger::ERROR);
\OC::$server->get(LoggerInterface::class)->error('unable to rename, source directory is not writable : ' . $srcParent, ['app' => 'core']);
return false;
}

if (!$this->isUpdatable($dstParent)) {
\OCP\Util::writeLog('core', 'unable to rename, destination directory is not writable : ' . $dstParent, ILogger::ERROR);
\OC::$server->get(LoggerInterface::class)->error('unable to rename, destination directory is not writable : ' . $dstParent, ['app' => 'core']);
return false;
}

if (!$this->file_exists($path1)) {
\OCP\Util::writeLog('core', 'unable to rename, file does not exists : ' . $path1, ILogger::ERROR);
\OC::$server->get(LoggerInterface::class)->error('unable to rename, file does not exists : ' . $path1, ['app' => 'core']);
return false;
}

Expand Down Expand Up @@ -484,7 +484,7 @@ public function getSourcePath($path) {
return $fullPath;
}

\OCP\Util::writeLog('core', "Following symlinks is not allowed ('$fullPath' -> '$realPath' not inside '{$this->realDataDir}')", ILogger::ERROR);
\OC::$server->get(LoggerInterface::class)->error("Following symlinks is not allowed ('$fullPath' -> '$realPath' not inside '{$this->realDataDir}')", ['app' => 'core']);
throw new ForbiddenException('Following symlinks is not allowed', false);
}

Expand Down