From f421fd25ba8a302ce394c35de287cb24c9f61202 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 9 Mar 2021 20:42:47 +0100 Subject: [PATCH] Move requesthandler controller to LoggerInterface * LoggerInterface * executeUpdate * log exception when there is one (just so we ahve the trace) Signed-off-by: Roeland Jago Douma --- .../Controller/RequestHandlerController.php | 6 +-- .../Controller/RequestHandlerController.php | 42 ++++++------------- .../RequestHandlerControllerTest.php | 6 +-- 3 files changed, 19 insertions(+), 35 deletions(-) diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index 17d2bea323..cc2364ed64 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -38,11 +38,11 @@ use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationProviderManager; use OCP\Federation\ICloudIdManager; use OCP\IGroupManager; -use OCP\ILogger; use OCP\IRequest; use OCP\IURLGenerator; use OCP\IUserManager; use OCP\Share\Exceptions\ShareNotFound; +use Psr\Log\LoggerInterface; /** * Class RequestHandlerController @@ -53,7 +53,7 @@ use OCP\Share\Exceptions\ShareNotFound; */ class RequestHandlerController extends Controller { - /** @var ILogger */ + /** @var LoggerInterface */ private $logger; /** @var IUserManager */ @@ -79,7 +79,7 @@ class RequestHandlerController extends Controller { public function __construct($appName, IRequest $request, - ILogger $logger, + LoggerInterface $logger, IUserManager $userManager, IGroupManager $groupManager, IURLGenerator $urlGenerator, diff --git a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php index ef49ca99ea..424a02d280 100644 --- a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php +++ b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php @@ -42,11 +42,11 @@ use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationProviderManager; use OCP\Federation\ICloudIdManager; use OCP\IDBConnection; -use OCP\ILogger; use OCP\IRequest; use OCP\IUserManager; use OCP\Share; use OCP\Share\Exceptions\ShareNotFound; +use Psr\Log\LoggerInterface; class RequestHandlerController extends OCSController { @@ -74,7 +74,7 @@ class RequestHandlerController extends OCSController { /** @var ICloudIdManager */ private $cloudIdManager; - /** @var ILogger */ + /** @var LoggerInterface */ private $logger; /** @var ICloudFederationFactory */ @@ -83,23 +83,7 @@ class RequestHandlerController extends OCSController { /** @var ICloudFederationProviderManager */ private $cloudFederationProviderManager; - /** - * Server2Server constructor. - * - * @param string $appName - * @param IRequest $request - * @param FederatedShareProvider $federatedShareProvider - * @param IDBConnection $connection - * @param Share\IManager $shareManager - * @param Notifications $notifications - * @param AddressHandler $addressHandler - * @param IUserManager $userManager - * @param ICloudIdManager $cloudIdManager - * @param ILogger $logger - * @param ICloudFederationFactory $cloudFederationFactory - * @param ICloudFederationProviderManager $cloudFederationProviderManager - */ - public function __construct($appName, + public function __construct(string $appName, IRequest $request, FederatedShareProvider $federatedShareProvider, IDBConnection $connection, @@ -108,7 +92,7 @@ class RequestHandlerController extends OCSController { AddressHandler $addressHandler, IUserManager $userManager, ICloudIdManager $cloudIdManager, - ILogger $logger, + LoggerInterface $logger, ICloudFederationFactory $cloudFederationFactory, ICloudFederationProviderManager $cloudFederationProviderManager ) { @@ -227,9 +211,9 @@ class RequestHandlerController extends OCSController { } catch (ProviderDoesNotExistsException $e) { throw new OCSException('Server does not support federated cloud sharing', 503); } catch (ShareNotFound $e) { - $this->logger->debug('Share not found: ' . $e->getMessage()); + $this->logger->debug('Share not found: ' . $e->getMessage(), ['exception' => $e]); } catch (\Exception $e) { - $this->logger->debug('internal server error, can not process notification: ' . $e->getMessage()); + $this->logger->debug('internal server error, can not process notification: ' . $e->getMessage(), ['exception' => $e]); } throw new OCSBadRequestException(); @@ -262,9 +246,9 @@ class RequestHandlerController extends OCSController { } catch (ProviderDoesNotExistsException $e) { throw new OCSException('Server does not support federated cloud sharing', 503); } catch (ShareNotFound $e) { - $this->logger->debug('Share not found: ' . $e->getMessage()); + $this->logger->debug('Share not found: ' . $e->getMessage(), ['exception' => $e]); } catch (\Exception $e) { - $this->logger->debug('internal server error, can not process notification: ' . $e->getMessage()); + $this->logger->debug('internal server error, can not process notification: ' . $e->getMessage(), ['exception' => $e]); } return new Http\DataResponse(); @@ -294,9 +278,9 @@ class RequestHandlerController extends OCSController { } catch (ProviderDoesNotExistsException $e) { throw new OCSException('Server does not support federated cloud sharing', 503); } catch (ShareNotFound $e) { - $this->logger->debug('Share not found: ' . $e->getMessage()); + $this->logger->debug('Share not found: ' . $e->getMessage(), ['exception' => $e]); } catch (\Exception $e) { - $this->logger->debug('internal server error, can not process notification: ' . $e->getMessage()); + $this->logger->debug('internal server error, can not process notification: ' . $e->getMessage(), ['exception' => $e]); } return new Http\DataResponse(); @@ -324,7 +308,7 @@ class RequestHandlerController extends OCSController { $notification = ['sharedSecret' => $token]; $provider->notificationReceived('SHARE_UNSHARED', $id, $notification); } catch (\Exception $e) { - $this->logger->debug('processing unshare notification failed: ' . $e->getMessage()); + $this->logger->debug('processing unshare notification failed: ' . $e->getMessage(), ['exception' => $e]); } return new Http\DataResponse(); @@ -398,7 +382,7 @@ class RequestHandlerController extends OCSController { $notification = ['sharedSecret' => $token, 'permission' => $ocmPermissions]; $provider->notificationReceived('RESHARE_CHANGE_PERMISSION', $id, $notification); } catch (\Exception $e) { - $this->logger->debug($e->getMessage()); + $this->logger->debug($e->getMessage(), ['exception' => $e]); throw new OCSBadRequestException(); } @@ -458,7 +442,7 @@ class RequestHandlerController extends OCSController { ->set('remote_id', $qb->createNamedParameter($newRemoteId)) ->where($qb->expr()->eq('remote_id', $qb->createNamedParameter($id))) ->andWhere($qb->expr()->eq('share_token', $qb->createNamedParameter($token))); - $affected = $query->execute(); + $affected = $query->executeUpdate(); if ($affected > 0) { return new Http\DataResponse(['remote' => $cloudId->getRemote(), 'owner' => $cloudId->getUser()]); diff --git a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php index 1b02d8d0b7..ff51a13b89 100644 --- a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php +++ b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php @@ -36,11 +36,11 @@ use OCP\Federation\ICloudFederationProviderManager; use OCP\Federation\ICloudFederationShare; use OCP\Federation\ICloudIdManager; use OCP\IDBConnection; -use OCP\ILogger; use OCP\IRequest; use OCP\IUserManager; use OCP\Share; use OCP\Share\IShare; +use Psr\Log\LoggerInterface; /** * Class RequestHandlerTest @@ -77,7 +77,7 @@ class RequestHandlerControllerTest extends \Test\TestCase { /** @var ICloudIdManager|\PHPUnit\Framework\MockObject\MockObject */ private $cloudIdManager; - /** @var ILogger|\PHPUnit\Framework\MockObject\MockObject */ + /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ private $logger; /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */ @@ -127,7 +127,7 @@ class RequestHandlerControllerTest extends \Test\TestCase { $this->cloudFederationShare = $this->createMock(ICloudFederationShare::class); - $this->logger = $this->createMock(ILogger::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->requestHandler = new RequestHandlerController( 'federatedfilesharing',