Migrate parts of files app away from depecrated Ilogger

- Migrate some backgrounds jobs, `TransferOwnership` & `DeleteOrphanedItems` - Migrate `DirectEditingController` Resolves : #32127 Signed-off-by: fenn-cs <[email protected]>
nextcloud · Jul 22, 2023 · cbcbc45 · cbcbc45
1 parent d0957ff
commit cbcbc45
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 79 deletions.
diff --git a/apps/files/lib/BackgroundJob/DeleteOrphanedItems.php b/apps/files/lib/BackgroundJob/DeleteOrphanedItems.php
@@ -27,34 +27,29 @@
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\TimedJob;
 use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IDBConnection;
+use Psr\Log\LoggerInterface;
 
 /**
  * Delete all share entries that have no matching entries in the file cache table.
  */
 class DeleteOrphanedItems extends TimedJob {
 	public const CHUNK_SIZE = 200;
-
-	/** @var \OCP\IDBConnection */
-	protected $connection;
-
-	/** @var \OCP\ILogger */
-	protected $logger;
-
-	/**
-	 * Default interval in minutes
-	 *
-	 * @var int $defaultIntervalMin
-	 **/
+	protected IDBConnection $connection;
+	protected LoggerInterface $logger;
 	protected $defaultIntervalMin = 60;
 
 	/**
 	 * sets the correct interval for this timed job
 	 */
-	public function __construct(ITimeFactory $time) {
+	public function __construct(
+		ITimeFactory $time,
+
+	) {
 		parent::__construct($time);
+		$this->connection = \OC::$server->get(IDBConnection::class);
+		$this->logger = \OC::$server->get(LoggerInterface::class);
 		$this->interval = $this->defaultIntervalMin * 60;
-		$this->connection = \OC::$server->getDatabaseConnection();
-		$this->logger = \OC::$server->getLogger();
 	}
 
 	/**

diff --git a/apps/files/lib/BackgroundJob/TransferOwnership.php b/apps/files/lib/BackgroundJob/TransferOwnership.php
@@ -33,46 +33,23 @@
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\QueuedJob;
 use OCP\Files\IRootFolder;
-use OCP\ILogger;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Notification\IManager as NotificationManager;
+use Psr\Log\LoggerInterface;
 use function ltrim;
 
 class TransferOwnership extends QueuedJob {
-
-	/** @var IUserManager $userManager */
-	private $userManager;
-
-	/** @var OwnershipTransferService */
-	private $transferService;
-
-	/** @var ILogger */
-	private $logger;
-
-	/** @var NotificationManager */
-	private $notificationManager;
-
-	/** @var TransferOwnershipMapper */
-	private $mapper;
-	/** @var IRootFolder */
-	private $rootFolder;
-
-	public function __construct(ITimeFactory $timeFactory,
-								IUserManager $userManager,
-								OwnershipTransferService $transferService,
-								ILogger $logger,
-								NotificationManager $notificationManager,
-								TransferOwnershipMapper $mapper,
-								IRootFolder $rootFolder) {
+	public function __construct(
+		ITimeFactory $timeFactory,
+		private IUserManager $userManager,
+		private OwnershipTransferService $transferService,
+		private LoggerInterface $logger,
+		private NotificationManager $notificationManager,
+		private TransferOwnershipMapper $mapper,
+		private IRootFolder $rootFolder,
+		) {
 		parent::__construct($timeFactory);
-
-		$this->userManager = $userManager;
-		$this->transferService = $transferService;
-		$this->logger = $logger;
-		$this->notificationManager = $notificationManager;
-		$this->mapper = $mapper;
-		$this->rootFolder = $rootFolder;
 	}
 
 	protected function run($argument) {
@@ -87,7 +64,10 @@ protected function run($argument) {
 		$nodes = $userFolder->getById($fileId);
 
 		if (empty($nodes)) {
-			$this->logger->alert('Could not transfer ownership: Node not found');
+			$this->logger->alert(
+				'Could not transfer ownership: Node not found',
+				['app' => 'files'],
+			);
 			$this->failedNotication($transfer);
 			return;
 		}
@@ -97,13 +77,19 @@ protected function run($argument) {
 		$destinationUserObject = $this->userManager->get($destinationUser);
 
 		if (!$sourceUserObject instanceof IUser) {
-			$this->logger->alert('Could not transfer ownership: Unknown source user ' . $sourceUser);
+			$this->logger->alert(
+				'Could not transfer ownership: Unknown source user ' . $sourceUser,
+				['app' => 'files'],
+			);
 			$this->failedNotication($transfer);
 			return;
 		}
 
 		if (!$destinationUserObject instanceof IUser) {
-			$this->logger->alert("Unknown destination user $destinationUser");
+			$this->logger->alert(
+				"Unknown destination user $destinationUser",
+				['app' => 'files'],
+			);
 			$this->failedNotication($transfer);
 			return;
 		}
@@ -116,7 +102,13 @@ protected function run($argument) {
 			);
 			$this->successNotification($transfer);
 		} catch (TransferOwnershipException $e) {
-			$this->logger->logException($e);
+			$this->logger->error(
+				$e->getMessage(),
+				[
+					'app' => 'files',
+					'exception' => $e,
+				],
+			);
 			$this->failedNotication($transfer);
 		}
 

diff --git a/apps/files/lib/Controller/DirectEditingController.php b/apps/files/lib/Controller/DirectEditingController.php
@@ -30,35 +30,24 @@
 use OCP\DirectEditing\IManager;
 use OCP\DirectEditing\RegisterDirectEditorEvent;
 use OCP\EventDispatcher\IEventDispatcher;
-use OCP\ILogger;
 use OCP\IRequest;
 use OCP\IURLGenerator;
+use Psr\Log\LoggerInterface;
 
 class DirectEditingController extends OCSController {
-	/** @var IEventDispatcher */
-	private $eventDispatcher;
-
-	/** @var IManager */
-	private $directEditingManager;
-
-	/** @var IURLGenerator */
-	private $urlGenerator;
-
-	/** @var ILogger */
-	private $logger;
-
-	/** @var DirectEditingService */
-	private $directEditingService;
-
-	public function __construct($appName, IRequest $request, $corsMethods, $corsAllowedHeaders, $corsMaxAge,
-								IEventDispatcher $eventDispatcher, IURLGenerator $urlGenerator, IManager $manager, DirectEditingService $directEditingService, ILogger $logger) {
+	public function __construct(
+		string $appName,
+		IRequest $request,
+		string $corsMethods,
+		string $corsAllowedHeaders,
+		int $corsMaxAge,
+		private IEventDispatcher $eventDispatcher,
+		private IURLGenerator $urlGenerator,
+		private IManager $directEditingManager,
+		private DirectEditingService $directEditingService,
+		private LoggerInterface $logger
+		) {
 		parent::__construct($appName, $request, $corsMethods, $corsAllowedHeaders, $corsMaxAge);
-
-		$this->eventDispatcher = $eventDispatcher;
-		$this->directEditingManager = $manager;
-		$this->directEditingService = $directEditingService;
-		$this->logger = $logger;
-		$this->urlGenerator = $urlGenerator;
 	}
 
 	/**
@@ -100,7 +89,13 @@ public function create(string $path, string $editorId, string $creatorId, string
 				'url' => $this->urlGenerator->linkToRouteAbsolute('files.DirectEditingView.edit', ['token' => $token])
 			]);
 		} catch (Exception $e) {
-			$this->logger->logException($e, ['message' => 'Exception when creating a new file through direct editing']);
+			$this->logger->error(
+				'Exception when creating a new file through direct editing',
+				[
+					'app' => 'files',
+					'exception' => $e
+				],
+			);
 			return new DataResponse(['message' => 'Failed to create file: ' . $e->getMessage()], Http::STATUS_FORBIDDEN);
 		}
 	}
@@ -131,7 +126,13 @@ public function open(string $path, string $editorId = null, ?int $fileId = null)
 				'url' => $this->urlGenerator->linkToRouteAbsolute('files.DirectEditingView.edit', ['token' => $token])
 			]);
 		} catch (Exception $e) {
-			$this->logger->logException($e, ['message' => 'Exception when opening a file through direct editing']);
+			$this->logger->error(
+				'Exception when opening a file through direct editing',
+				[
+					'app' => 'files',
+					'exception' => $e
+				],
+			);
 			return new DataResponse(['message' => 'Failed to open file: ' . $e->getMessage()], Http::STATUS_FORBIDDEN);
 		}
 	}
@@ -159,7 +160,13 @@ public function templates(string $editorId, string $creatorId): DataResponse {
 		try {
 			return new DataResponse($this->directEditingManager->getTemplates($editorId, $creatorId));
 		} catch (Exception $e) {
-			$this->logger->logException($e);
+			$this->logger->error(
+				$e->getMessage(),
+				[
+					'app' => 'files',
+					'exception' => $e
+				],
+			);
 			return new DataResponse(['message' => 'Failed to obtain template list: ' . $e->getMessage()], Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 	}