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`

Signed-off-by: fenn-cs <[email protected]>
  • Loading branch information
@nfebe
nfebe committed Aug 8, 2023
1 parent bfe42de commit cb55dee
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 85 deletions.
22 changes: 7 additions & 15 deletions apps/files/lib/BackgroundJob/DeleteOrphanedItems.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,34 +27,26 @@
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 $defaultIntervalMin = 60;

/**
* sets the correct interval for this timed job
*/
public function __construct(ITimeFactory $time) {
public function __construct(
ITimeFactory $time,
protected IDBConnection $connection,
protected LoggerInterface $logger,
) {
parent::__construct($time);
$this->interval = $this->defaultIntervalMin * 60;
$this->connection = \OC::$server->getDatabaseConnection();
$this->logger = \OC::$server->getLogger();
}

/**
Expand Down
51 changes: 16 additions & 35 deletions apps/files/lib/BackgroundJob/TransferOwnership.php
View file
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 Down Expand Up @@ -116,7 +93,12 @@ protected function run($argument) {
);
$this->successNotification($transfer);
} catch (TransferOwnershipException $e) {
$this->logger->logException($e);
$this->logger->error(
$e->getMessage(),
[
'exception' => $e,
],
);
$this->failedNotication($transfer);
}

Expand All @@ -136,7 +118,6 @@ private function failedNotication(Transfer $transfer): void {
])
->setObject('transfer', (string)$transfer->getId());
$this->notificationManager->notify($notification);

// Send notification to source user
$notification = $this->notificationManager->createNotification();
$notification->setUser($transfer->getTargetUser())
Expand Down
58 changes: 31 additions & 27 deletions apps/files/lib/Controller/DirectEditingController.php
View file
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 Down Expand Up @@ -100,7 +89,12 @@ 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',
[
'exception' => $e
],
);
return new DataResponse(['message' => 'Failed to create file: ' . $e->getMessage()], Http::STATUS_FORBIDDEN);
}
}
Expand Down Expand Up @@ -131,7 +125,12 @@ 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',
[
'exception' => $e
],
);
return new DataResponse(['message' => 'Failed to open file: ' . $e->getMessage()], Http::STATUS_FORBIDDEN);
}
}
Expand Down Expand Up @@ -159,7 +158,12 @@ 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(),
[
'exception' => $e
],
);
return new DataResponse(['message' => 'Failed to obtain template list: ' . $e->getMessage()], Http::STATUS_INTERNAL_SERVER_ERROR);
}
}
Expand Down
18 changes: 10 additions & 8 deletions apps/files/tests/BackgroundJob/DeleteOrphanedItemsJobTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
use OCA\Files\BackgroundJob\DeleteOrphanedItems;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
use Psr\Log\LoggerInterface;

/**
* Class DeleteOrphanedItemsJobTest
Expand All @@ -35,15 +37,15 @@
* @package Test\BackgroundJob
*/
class DeleteOrphanedItemsJobTest extends \Test\TestCase {
/** @var \OCP\IDBConnection */
protected $connection;

protected IDBConnection $connection;
protected LoggerInterface $logger;
protected ITimeFactory $timeFactory;

protected function setUp(): void {
parent::setUp();
$this->connection = \OC::$server->getDatabaseConnection();
$this->connection = \OC::$server->get(IDBConnection::class);
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->logger = \OC::$server->get(LoggerInterface::class);
}

protected function cleanMapping($table) {
Expand Down Expand Up @@ -98,7 +100,7 @@ public function testClearSystemTagMappings() {
$mapping = $this->getMappings('systemtag_object_mapping');
$this->assertCount(2, $mapping);

$job = new DeleteOrphanedItems($this->timeFactory);
$job = new DeleteOrphanedItems($this->timeFactory, $this->connection, $this->logger);
$this->invokePrivate($job, 'cleanSystemTags');

$mapping = $this->getMappings('systemtag_object_mapping');
Expand Down Expand Up @@ -147,7 +149,7 @@ public function testClearUserTagMappings() {
$mapping = $this->getMappings('vcategory_to_object');
$this->assertCount(2, $mapping);

$job = new DeleteOrphanedItems($this->timeFactory);
$job = new DeleteOrphanedItems($this->timeFactory, $this->connection, $this->logger);
$this->invokePrivate($job, 'cleanUserTags');

$mapping = $this->getMappings('vcategory_to_object');
Expand Down Expand Up @@ -198,7 +200,7 @@ public function testClearComments() {
$mapping = $this->getMappings('comments');
$this->assertCount(2, $mapping);

$job = new DeleteOrphanedItems($this->timeFactory);
$job = new DeleteOrphanedItems($this->timeFactory, $this->connection, $this->logger);
$this->invokePrivate($job, 'cleanComments');

$mapping = $this->getMappings('comments');
Expand Down Expand Up @@ -247,7 +249,7 @@ public function testClearCommentReadMarks() {
$mapping = $this->getMappings('comments_read_markers');
$this->assertCount(2, $mapping);

$job = new DeleteOrphanedItems($this->timeFactory);
$job = new DeleteOrphanedItems($this->timeFactory, $this->connection, $this->logger);
$this->invokePrivate($job, 'cleanCommentMarkers');

$mapping = $this->getMappings('comments_read_markers');
Expand Down

0 comments on commit cb55dee

Please sign in to comment.