Skip to content

Commit

Permalink
Migrate parts of files app away from depecrated Ilogger
Browse files Browse the repository at this point in the history
- Migrate some backgrounds jobs, `TransferOwnership` & `DeleteOrphanedItems`
- Migrate `DirectEditingController`

Resolves :  #32127

Signed-off-by: fenn-cs <[email protected]>
  • Loading branch information
Fenn-CS committed Jul 13, 2023
1 parent a9ba19d commit 9351b1c
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 78 deletions.
25 changes: 11 additions & 14 deletions apps/files/lib/BackgroundJob/DeleteOrphanedItems.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,34 +27,31 @@
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;
protected IDBConnection $connection;

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

/**
* Default interval in minutes
*
* @var int $defaultIntervalMin
**/
protected $defaultIntervalMin = 60;
protected LoggerInterface $logger;

/**
* sets the correct interval for this timed job
*/
public function __construct(ITimeFactory $time) {
public function __construct(
ITimeFactory $time,

protected int $defaultIntervalMin = 60
) {
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();
}

/**
Expand Down
66 changes: 29 additions & 37 deletions apps/files/lib/BackgroundJob/TransferOwnership.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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);
}

Expand Down
61 changes: 34 additions & 27 deletions apps/files/lib/Controller/DirectEditingController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -85,7 +74,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);
}
}
Expand All @@ -105,7 +100,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);
}
}
Expand All @@ -124,7 +125,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);
}
}
Expand Down

0 comments on commit 9351b1c

Please sign in to comment.