diff --git a/.drone.yml b/.drone.yml index 18b1322..6f5b0c4 100644 --- a/.drone.yml +++ b/.drone.yml @@ -15,10 +15,128 @@ pipeline: - bash ./before_install.sh $APP_NAME $CORE_BRANCH $DB - cd ../server - php ./build/signed-off-checker.php - secrets: [ github_token ] when: matrix: TESTS: signed-off-check + check-app-compatbility: + image: nextcloudci/php7.0:php7.0-2 + environment: + - APP_NAME=data_request + - CORE_BRANCH=stable13 + - DB=sqlite + commands: + # Pre-setup steps + - wget https://raw.githubusercontent.com/nextcloud/travis_ci/master/before_install.sh + - bash ./before_install.sh $APP_NAME $CORE_BRANCH $DB + - cd ../server + + # Code checker + - ./occ app:check-code $APP_NAME + - cd apps/$APP_NAME/ + when: + matrix: + TESTS: check-app-compatbility + syntax-php5.6: + image: nextcloudci/php5.6:php5.6-8 + environment: + - APP_NAME=data_request + - CORE_BRANCH=stable13 + - DB=sqlite + commands: + # Pre-setup steps + - wget https://raw.githubusercontent.com/nextcloud/travis_ci/master/before_install.sh + - bash ./before_install.sh $APP_NAME $CORE_BRANCH $DB + - cd ../server + - composer install + - ./lib/composer/bin/parallel-lint apps/$APP_NAME/ + when: + matrix: + TESTS: syntax-php5.6 + syntax-php7.0: + image: nextcloudci/php7.0:php7.0-2 + environment: + - APP_NAME=data_request + - CORE_BRANCH=stable13 + - DB=sqlite + commands: + # Pre-setup steps + - wget https://raw.githubusercontent.com/nextcloud/travis_ci/master/before_install.sh + - bash ./before_install.sh $APP_NAME $CORE_BRANCH $DB + - cd ../server + - composer install + - ./lib/composer/bin/parallel-lint apps/$APP_NAME/ + when: + matrix: + TESTS: syntax-php7.0 + php5.6: + image: nextcloudci/php5.6:php5.6-8 + environment: + - APP_NAME=data_request + - CORE_BRANCH=stable13 + - DB=sqlite + commands: + - apt update && apt-get -y install php5-xdebug + + # Pre-setup steps + - wget https://raw.githubusercontent.com/nextcloud/travis_ci/master/before_install.sh + - bash ./before_install.sh $APP_NAME $CORE_BRANCH $DB + - cd ../server/ + - ./occ app:enable $APP_NAME + - cd apps/$APP_NAME + + # Run phpunit tests + - cd tests/unit/ + - phpunit --configuration phpunit.xml + when: + matrix: + TESTS: php5.6 + php7.0: + image: nextcloudci/php7.0:php7.0-2 + environment: + - APP_NAME=data_request + - CORE_BRANCH=stable13 + - DB=sqlite + commands: + # Pre-setup steps + - wget https://raw.githubusercontent.com/nextcloud/travis_ci/master/before_install.sh + - bash ./before_install.sh $APP_NAME $CORE_BRANCH $DB + - cd ../server/ + - ./occ app:enable $APP_NAME + - cd apps/$APP_NAME + + # Run phpunit tests + - cd tests/unit/ + - phpunit --configuration phpunit.xml + when: + matrix: + TESTS: php7.0 + php7.1: + image: nextcloudci/php7.1:php7.1-15 + environment: + - APP_NAME=data_request + - CORE_BRANCH=stable13 + - DB=sqlite + commands: + # Pre-setup steps + - wget https://raw.githubusercontent.com/nextcloud/travis_ci/master/before_install.sh + - bash ./before_install.sh $APP_NAME $CORE_BRANCH $DB + - cd ../server/ + - ./occ app:enable $APP_NAME + - cd apps/$APP_NAME + + # Run phpunit tests + - cd tests/unit/ + - phpunit --configuration phpunit.xml + when: + matrix: + TESTS: php7.1 + matrix: include: - TESTS: signed-off-check + - TESTS: check-app-compatbility + - TESTS: syntax-php5.6 + - TESTS: syntax-php7.0 + - TESTS: php5.6 + - TESTS: php7.0 + - TESTS: php7.1 diff --git a/lib/Controller/DataRequestController.php b/lib/Controller/DataRequestController.php index 60b15de..c7477cf 100644 --- a/lib/Controller/DataRequestController.php +++ b/lib/Controller/DataRequestController.php @@ -52,15 +52,9 @@ class DataRequestController extends OCSController { * @PasswordConfirmationRequired */ public function export() { - try { + return $this->processRequest(function() { $this->dataRequest->sendExportRequest(); - return new DataResponse(); - } catch(HintedRuntime $e) { - return new DataResponse( - ['error' => $e->getHint()], - Http::STATUS_INTERNAL_SERVER_ERROR - ); - } + }); } /** @@ -68,8 +62,14 @@ class DataRequestController extends OCSController { * @PasswordConfirmationRequired */ public function deletion() { - try { + return $this->processRequest(function() { $this->dataRequest->sendDeleteRequest(); + }); + } + + protected function processRequest(callable $serviceMethod) { + try { + $serviceMethod(); return new DataResponse(); } catch(HintedRuntime $e) { return new DataResponse( diff --git a/lib/Services/Request.php b/lib/Services/Request.php index 7fa8611..e6fa047 100644 --- a/lib/Services/Request.php +++ b/lib/Services/Request.php @@ -50,14 +50,25 @@ class Request { private $requester; /** @var IL10N */ private $l; + /** @var Defaults */ + private $defaults; - public function __construct(IGroupManager $groupManager, IMailer $mailer, IFactory $l10nFactory, IConfig $config, IUserSession $userSession, IL10N $l) { + public function __construct( + IGroupManager $groupManager, + IMailer $mailer, + IFactory $l10nFactory, + IConfig $config, + IUserSession $userSession, + IL10N $l, + Defaults $defaults + ) { $this->groupManager = $groupManager; $this->mailer = $mailer; $this->l10nFactory = $l10nFactory; $this->config = $config; $this->requester = $userSession->getUser(); $this->l = $l; + $this->defaults = $defaults; } public function sendExportRequest() { @@ -95,21 +106,23 @@ class Request { } protected function craftEmailTo(IUser $admin, IEMailTemplate $template) { - $defaults = new Defaults(); $senderAddress = Util::getDefaultEmailAddress('no-reply'); - $senderName = $defaults->getName(); + $senderName = $this->defaults->getName(); $message = $this->mailer->createMessage(); - $message->setTo([$admin->getEMailAddress() => $admin->getDisplayName()]); + $message->setTo([$admin->getEMailAddress () => $admin->getDisplayName()]); $message->setSubject($template->renderSubject()); $message->setHtmlBody($template->renderHtml()); $message->setPlainBody($template->renderText()); $message->setFrom([$senderAddress => $senderName]); try { - $this->mailer->send($message); + $failedRecipients = $this->mailer->send($message); + if(count($failedRecipients) > 0) { + return false; + } } catch (\Exception $e) { - return $e; + return false; } return true; @@ -147,7 +160,7 @@ class Request { protected function getAdmins() { $admins = $this->groupManager->get('admin')->searchUsers(''); - $admins = array_filter($admins, function(IUser $admin) { + $admins = array_filter($admins, function(IUser $admin) { return $admin->getEMailAddress() !== null; }); if(empty($admins)) { diff --git a/tests/unit/Controller/DataRequestControllerTest.php b/tests/unit/Controller/DataRequestControllerTest.php new file mode 100644 index 0000000..49793fb --- /dev/null +++ b/tests/unit/Controller/DataRequestControllerTest.php @@ -0,0 +1,120 @@ + + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\DataRequest\Tests\unit\Controller; + +use OCA\DataRequest\Controller\DataRequestController; +use OCA\DataRequest\Exceptions\HintedRuntime; +use OCA\DataRequest\Services\Request; +use OCP\AppFramework\Http\DataResponse; +use OCP\IRequest; + +class DataRequestControllerTest extends \Test\TestCase { + + /** @var Request|\PHPUnit_Framework_MockObject_MockObject */ + protected $requestService; + + /** @var DataRequestController */ + protected $controller; + + public function setUp() { + parent::setUp(); + + $this->requestService = $this->createMock(Request::class); + + /** @var IRequest $request */ + $request = $this->createMock(IRequest::class); + + $this->controller = new DataRequestController( + 'data_request', + $request, + 'PUT, POST, GET, DELETE, PATCH', + 'Authorization, Content-Type, Accept', + 1728000, + $this->requestService + ); + } + + public function processDataProvider() { + return [ + [ + 'sendExportRequest', + true, + 500 + ], + [ + 'sendExportRequest', + false, + 200 + ], + [ + 'sendDeleteRequest', + true, + 500 + ], + [ + 'sendDeleteRequest', + false, + 200 + ] + ]; + } + + /** + * @dataProvider processDataProvider + * @param string $serviceMethod + * @param bool $causeException + * @param int $expectStatus + */ + public function testRequests($serviceMethod, $causeException, $expectStatus) { + $mocker = $this->requestService->expects($this->once()) + ->method($serviceMethod); + + if($causeException) { + /** @var HintedRuntime|\PHPUnit_Framework_MockObject_MockObject $exception */ + $exception = $this->createMock(HintedRuntime::class); + $exception->expects($this->once()) + ->method('getHint') + ->willReturn('Some hint'); + + $mocker->willThrowException($exception); + } + + switch($serviceMethod) { + case 'sendExportRequest': + $response = $this->controller->export(); + break; + case 'sendDeleteRequest': + $response = $this->controller->deletion(); + break; + default: + throw new \RuntimeException('No valid test case'); + } + + $this->assertInstanceOf(DataResponse::class, $response); + $this->assertSame($expectStatus, $response->getStatus()); + if($expectStatus >= 500) { + $this->assertSame('Some hint', $response->getData()['error']); + } + } +} diff --git a/tests/unit/Services/RequestTest.php b/tests/unit/Services/RequestTest.php new file mode 100644 index 0000000..b712351 --- /dev/null +++ b/tests/unit/Services/RequestTest.php @@ -0,0 +1,297 @@ + + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +class RequestTest extends \Test\TestCase { + /** @var IGroupManager|PHPUnit_Framework_MockObject_MockObject */ + protected $groupManager; + /** @var IMailer|PHPUnit_Framework_MockObject_MockObject */ + protected $mailer; + /** @var IFactory|PHPUnit_Framework_MockObject_MockObject */ + protected $l10nFactory; + /** @var IConfig|PHPUnit_Framework_MockObject_MockObject */ + protected $config; + /** @var IUserSession|PHPUnit_Framework_MockObject_MockObject */ + protected $session; + /** @var IL10N|PHPUnit_Framework_MockObject_MockObject */ + protected $l; + /** @var Request */ + protected $service; + /** @var IUser|PHPUnit_Framework_MockObject_MockObject */ + protected $user; + /** @var Defaults|PHPUnit_Framework_MockObject_MockObject */ + protected $defaults; + + public function setUp() { + parent::setUp(); + + $this->groupManager = $this->createMock(IGroupManager::class); + $this->mailer = $this->createMock(IMailer::class); + $this->l10nFactory = $this->createMock(IFactory::class); + $this->config = $this->createMock(IConfig::class); + + $this->user = $this->createMock(IUser::class); + + $this->session = $this->createMock(IUserSession::class); + $this->session->expects($this->any()) + ->method('getUser') + ->willReturn($this->user); + + $this->l = $this->createMock(IL10N::class); + + $this->defaults = $this->createMock(Defaults::class); + + $this->service = new Request( + $this->groupManager, + $this->mailer, + $this->l10nFactory, + $this->config, + $this->session, + $this->l, + $this->defaults + ); + } + + public function templateProvider() { + return [ + [ + 'getExportTemplate', + 'data_request.Export', + 'Personal data export request' + ], + [ + 'getDeletionTemplate', + 'data_request.Deletion', + 'Account deletion request' + ] + ]; + } + + /** + * @dataProvider templateProvider + * @param $method + * @param $templateId + * @param $expectedSubject + */ + public function testTemplates($method, $templateId, $expectedSubject) { + $adminUid = 'elu-thingol'; + $adminName = 'Elu Thingol'; + $adminLang = 'qya'; + $admin = $this->createMock(IUser::class); + $admin->expects($this->any()) + ->method('getUID') + ->willReturn($adminUid); + $admin->expects($this->any()) + ->method('getDisplayName') + ->willReturn($adminName); + + $this->config->expects($this->atLeastOnce()) + ->method('getSystemValue') + ->with('default_language') + ->willReturn('tlh'); + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with($adminUid, 'core', 'lang', 'tlh') + ->willReturn($adminLang); + + $l = $this->createMock(IL10N::class); + $l->expects($this->atLeast(3)) + ->method('t') + ->willReturnArgument(0); + + $this->l10nFactory->expects($this->once()) + ->method('get') + ->with('data_request', $adminLang) + ->willReturn($l); + + $template = $this->createMock(IEMailTemplate::class); + $template->expects($this->once()) + ->method('setSubject') + ->with($expectedSubject); + $template->expects($this->once()) + ->method('addHeader'); + $template->expects($this->once()) + ->method('addHeading'); + $template->expects($this->once()) + ->method('addBodyText'); + $template->expects($this->once()) + ->method('addFooter'); + + $this->mailer->expects($this->once()) + ->method('createEMailTemplate') + ->with($templateId, []) + ->willReturn($template); + + $result = $this->invokePrivate($this->service, $method, [$admin]); + + $this->assertSame($template, $result); + } + + + public function adminProvider() { + $admin1 = $this->createMock(IUser::class); + + $admin2 = $this->createMock(IUser::class); + $admin2->expects($this->any()) + ->method('getEMailAddress') + ->willReturn('admin2@sindar.gov'); + + $admin3 = $this->createMock(IUser::class); + $admin3->expects($this->any()) + ->method('getEMailAddress') + ->willReturn('admin3@sindar.gov'); + + $admin4 = $this->createMock(IUser::class); + + $admin5 = $this->createMock(IUser::class); + $admin5->expects($this->any()) + ->method('getEMailAddress') + ->willReturn('admin5@sindar.gov'); + + return [ + [ + [ $admin1 ], + 0 + ], + [ + [ $admin2 ], + 1 + ], + [ + [ $admin3, $admin4, $admin5 ], // for whatever reasons, reusing $admin1 and $admin2 would fail on CI + 2 + ] + ]; + } + + /** + * @dataProvider adminProvider + * @param $admins + * @param $adminsWithEmail + */ + public function testGetAdmins($admins, $adminsWithEmail) { + $adminGroup = $this->createMock(IGroup::class); + $adminGroup->expects($this->once()) + ->method('searchUsers') + ->with('') + ->willReturn($admins); + + $this->groupManager->expects($this->once()) + ->method('get') + ->with('admin') + ->willReturn($adminGroup); + + if($adminsWithEmail === 0) { + $this->expectException(HintedRuntime::class); + } + $result = $this->invokePrivate($this->service, 'getAdmins'); + + $this->assertSame($adminsWithEmail, count($result)); + } + + public function mailerSendProvider() { + return [ + [ + false, [], true + ], + [ + false, ['elu-thingol@sindar.gov'], false + ], + [ + true, [], false + ] + ]; + } + + /** + * @dataProvider mailerSendProvider + * @param $sendThrowsException + * @param $sendResult + * @param $expectedResult + */ + public function testSendMail($sendThrowsException, $sendResult, $expectedResult) { + $adminName = 'Elu Thingol'; + $adminMail = 'elu-thingol@sindar.gov'; + $admin = $this->createMock(IUser::class); + $admin->expects($this->any()) + ->method('getEMailAddress') + ->willReturn($adminMail); + $admin->expects($this->any()) + ->method('getDisplayName') + ->willReturn($adminName); + + $template = $this->createMock(IEMailTemplate::class); + $template->expects($this->once()) + ->method('renderSubject'); + $template->expects($this->once()) + ->method('renderHtml'); + $template->expects($this->once()) + ->method('renderText'); + + $message = $this->createMock(\OC\Mail\Message::class); + $message->expects($this->once()) + ->method('setTo') + ->with([$adminMail => $adminName]); + $message->expects($this->once()) + ->method('setSubject'); + $message->expects($this->once()) + ->method('setHtmlBody'); + $message->expects($this->once()) + ->method('setPlainBody'); + $message->expects($this->once()) + ->method('setFrom'); + + $this->mailer->expects($this->once()) + ->method('createMessage') + ->willReturn($message); + + $sendMocker = $this->mailer->expects($this->once()) + ->method('send') + ->with($message); + if($sendThrowsException) { + $sendMocker->willThrowException(new \Exception('Expected Exception')); + } else { + $sendMocker->willReturn($sendResult); + } + + $this->defaults->expects($this->atLeastOnce()) + ->method('getName') + ->willReturn('Cloud of Sindar'); + + $result = $this->invokePrivate($this->service, 'craftEmailTo', [$admin, $template]); + $this->assertSame($expectedResult, $result); + } +} diff --git a/tests/unit/bootstrap.php b/tests/unit/bootstrap.php new file mode 100644 index 0000000..cbfa0d9 --- /dev/null +++ b/tests/unit/bootstrap.php @@ -0,0 +1,28 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +if (!defined('PHPUNIT_RUN')) { + define('PHPUNIT_RUN', 1); +} +require_once __DIR__.'/../../../../lib/base.php'; +\OC::$loader->addValidRoot(\OC::$SERVERROOT . '/tests'); +\OC_App::loadApp('data_request'); +OC_Hook::clear(); diff --git a/tests/unit/phpunit.xml b/tests/unit/phpunit.xml new file mode 100644 index 0000000..9c447bc --- /dev/null +++ b/tests/unit/phpunit.xml @@ -0,0 +1,23 @@ + + + + . + + + + + ../../../data_request/appinfo + ../../../data_request/lib + + + + + + +