diff --git a/application/api/controllers/Links.php b/application/api/controllers/Links.php index 578cec3a3..55104a2c7 100644 --- a/application/api/controllers/Links.php +++ b/application/api/controllers/Links.php @@ -2,7 +2,6 @@ namespace Shaarli\Api\Controllers; -use DI\Container; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Shaarli\Api\ApiUtils; @@ -25,11 +24,6 @@ class Links extends ApiController */ public static $DEFAULT_LIMIT = 20; - public function __construct(Container $ci) - { - parent::__construct($ci); - } - /** * Retrieve a list of bookmarks, allowing different filters. * diff --git a/application/api/exceptions/ApiException.php b/application/api/exceptions/ApiException.php index ce3d750ae..0386b51de 100644 --- a/application/api/exceptions/ApiException.php +++ b/application/api/exceptions/ApiException.php @@ -2,7 +2,6 @@ namespace Shaarli\Api\Exceptions; -use Slim\Http\Response; use Slim\Psr7\Response as SlimResponse; /** diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php index 4e23e3b00..08bcb24f4 100644 --- a/application/container/ContainerBuilder.php +++ b/application/container/ContainerBuilder.php @@ -6,14 +6,13 @@ use DI\Container; use malkusch\lock\mutex\FlockMutex; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Shaarli\Bookmark\BookmarkFileService; use Shaarli\Bookmark\BookmarkServiceInterface; use Shaarli\Config\ConfigManager; use Shaarli\Feed\FeedBuilder; use Shaarli\Formatter\FormatterFactory; -use Shaarli\Front\Controller\Visitor\ErrorController; -use Shaarli\Front\Controller\Visitor\ErrorNotFoundController; use Shaarli\History; use Shaarli\Http\HttpAccess; use Shaarli\Http\MetadataRetriever; @@ -76,7 +75,7 @@ public function __construct( $this->logger = $logger; } - public function build(): Container + public function build(): ContainerInterface { $container = new Container(); diff --git a/application/front/ShaarliAdminMiddleware.php b/application/front/ShaarliAdminMiddleware.php index 807e6e90e..d2a72bf51 100644 --- a/application/front/ShaarliAdminMiddleware.php +++ b/application/front/ShaarliAdminMiddleware.php @@ -18,7 +18,7 @@ public function __invoke(Request $request, RequestHandler $handler): Response $this->initBasePath($request); if (true !== $this->container->get('loginManager')->isLoggedIn()) { - $returnUrl = urlencode($request->getServerParams()['REQUEST_URI'] ?? null); + $returnUrl = urlencode($request->getServerParams()['REQUEST_URI']); return $this->redirect($this->container->get('basePath') . '/login?returnurl=' . $returnUrl); } diff --git a/application/front/ShaarliErrorHandler.php b/application/front/ShaarliErrorHandler.php index a98a2f6d9..84adcc46e 100644 --- a/application/front/ShaarliErrorHandler.php +++ b/application/front/ShaarliErrorHandler.php @@ -9,19 +9,20 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerInterface; -use Shaarli\Bookmark\BookmarkFilter; +use Shaarli\Front\Controller\PageTrait; use Shaarli\Front\Exception\ShaarliFrontException; use Shaarli\Render\TemplatePage; use Slim\Exception\HttpNotFoundException; use Slim\Handlers\ErrorHandler; use Slim\Interfaces\CallableResolverInterface; -use Slim\Psr7\Response; use Slim\Psr7\Response as SlimResponse; use Slim\Routing\RouteContext; use Throwable; class ShaarliErrorHandler extends ErrorHandler { + use PageTrait; + private ?Container $container; public function __construct( @@ -41,8 +42,8 @@ public function __invoke( bool $logErrors, bool $logErrorDetails ): ResponseInterface { - $resp = parent::__invoke($request, $exception, $displayErrorDetails, $logErrors, $logErrorDetails); - $response = new Response(); + parent::__invoke($request, $exception, $displayErrorDetails, $logErrors, $logErrorDetails); + $response = new SlimResponse(); // Unknown error encountered $this->container->get('pageBuilder')->reset(); if ($exception instanceof HttpNotFoundException) { @@ -72,74 +73,11 @@ public function __invoke( $response = $response->withStatus(500); } - $response->getBody()->write($this->render(TemplatePage::ERROR)); return $response; } - protected function assignView(string $name, $value): self - { - $this->container->get('pageBuilder')->assign($name, $value); - - return $this; - } - - /** - * Call plugin hooks for header, footer and includes, specifying which page will be rendered. - * Then assign generated data to RainTPL. - */ - protected function executeDefaultHooks(string $template): void - { - $common_hooks = [ - 'includes', - 'header', - 'footer', - ]; - - $parameters = $this->buildPluginParameters($template); - - foreach ($common_hooks as $name) { - $pluginData = []; - $this->container->get('pluginManager')->executeHooks( - 'render_' . $name, - $pluginData, - $parameters - ); - $this->assignView('plugins_' . $name, $pluginData); - } - } - - protected function buildPluginParameters(?string $template): array - { - $basePath = $this->container->get('basePath') ?? ''; - return [ - 'target' => $template, - 'loggedin' => $this->container->get('loginManager')->isLoggedIn(), - 'basePath' => $this->container->get('basePath'), - 'rootPath' => preg_replace('#/index\.php$#', '', $basePath), - 'bookmarkService' => $this->container->get('bookmarkService') - ]; - } - - protected function render(string $template): string - { - // Legacy key that used to be injected by PluginManager - $this->assignView('_PAGE_', $template); - $this->assignView('template', $template); - - $this->assignView('linkcount', $this->container->get('bookmarkService')->count(BookmarkFilter::$ALL)); - $this->assignView('privateLinkcount', $this->container->get('bookmarkService') - ->count(BookmarkFilter::$PRIVATE)); - - $this->executeDefaultHooks($template); - - $this->assignView('plugin_errors', $this->container->get('pluginManager')->getErrors()); - - $basePath = $this->container->get('basePath') ?? ''; - return $this->container->get('pageBuilder')->render($template, $basePath); - } - - protected function showError404($request): Response + protected function showError404($request): ResponseInterface { $response = new SlimResponse(); // Request from the API diff --git a/application/front/ShaarliMiddleware.php b/application/front/ShaarliMiddleware.php index b5a1c01aa..1224d22fa 100644 --- a/application/front/ShaarliMiddleware.php +++ b/application/front/ShaarliMiddleware.php @@ -56,7 +56,7 @@ public function __invoke(Request $request, RequestHandler $handler): Response return $handler->handle($request); } catch (UnauthorizedException $e) { - $returnUrl = urlencode($request->getServerParams()['REQUEST_URI'] ?? null); + $returnUrl = urlencode($request->getServerParams()['REQUEST_URI']); return $this->redirect($this->container->get('basePath') . '/login?returnurl=' . $returnUrl); } diff --git a/application/front/controller/PageTrait.php b/application/front/controller/PageTrait.php new file mode 100644 index 000000000..82ccfbbd3 --- /dev/null +++ b/application/front/controller/PageTrait.php @@ -0,0 +1,72 @@ +container->get('pageBuilder')->assign($name, $value); + + return $this; + } + + /** + * Call plugin hooks for header, footer and includes, specifying which page will be rendered. + * Then assign generated data to RainTPL. + */ + protected function executeDefaultHooks(string $template): void + { + $common_hooks = [ + 'includes', + 'header', + 'footer', + ]; + + $parameters = $this->buildPluginParameters($template); + + foreach ($common_hooks as $name) { + $pluginData = []; + $this->container->get('pluginManager')->executeHooks( + 'render_' . $name, + $pluginData, + $parameters + ); + $this->assignView('plugins_' . $name, $pluginData); + } + } + + protected function buildPluginParameters(?string $template): array + { + $basePath = $this->container->get('basePath') ?? ''; + return [ + 'target' => $template, + 'loggedin' => $this->container->get('loginManager')->isLoggedIn(), + 'basePath' => $this->container->get('basePath'), + 'rootPath' => preg_replace('#/index\.php$#', '', $basePath), + 'bookmarkService' => $this->container->get('bookmarkService') + ]; + } + + protected function render(string $template): string + { + // Legacy key that used to be injected by PluginManager + $this->assignView('_PAGE_', $template); + $this->assignView('template', $template); + + $this->assignView('linkcount', $this->container->get('bookmarkService')->count(BookmarkFilter::$ALL)); + $this->assignView('privateLinkcount', $this->container->get('bookmarkService') + ->count(BookmarkFilter::$PRIVATE)); + + $this->executeDefaultHooks($template); + + $this->assignView('plugin_errors', $this->container->get('pluginManager')->getErrors()); + + $basePath = $this->container->get('basePath') ?? ''; + return $this->container->get('pageBuilder')->render($template, $basePath); + } +} diff --git a/application/front/controller/visitor/ErrorController.php b/application/front/controller/visitor/ErrorController.php deleted file mode 100644 index d3dde12fd..000000000 --- a/application/front/controller/visitor/ErrorController.php +++ /dev/null @@ -1,51 +0,0 @@ -container->get('pageBuilder')->reset(); - - if ($throwable instanceof ShaarliFrontException) { - // Functional error - $this->assignView('message', nl2br($throwable->getMessage())); - - $response = $response->withStatus($throwable->getCode()); - } else { - // Internal error (any other Throwable) - if ( - $this->container->get('conf')->get('dev.debug', false) || - $this->container->get('loginManager')->isLoggedIn() - ) { - $this->assignView('message', t('Error: ') . $throwable->getMessage()); - $this->assignView( - 'text', - '' - . t('Please report it on Github.') - . '' - ); - $this->assignView('stacktrace', exception2text($throwable)); - } else { - $this->assignView('message', t('An unexpected error occurred.')); - } - - $response = $response->withStatus(500); - } - - return $this->respondWithTemplate($response, TemplatePage::ERROR); - } -} diff --git a/application/front/controller/visitor/ErrorNotFoundController.php b/application/front/controller/visitor/ErrorNotFoundController.php deleted file mode 100644 index 7fd30916d..000000000 --- a/application/front/controller/visitor/ErrorNotFoundController.php +++ /dev/null @@ -1,35 +0,0 @@ -getRequestTarget(), '/api/v1')) { - return $response->withStatus(404); - } - $basePathFromRequest = $request->getAttribute(RouteContext::BASE_PATH); - - // This is required because the middleware is ignored if the route is not found. - $this->container->set('basePath', rtrim($basePathFromRequest, '/')); - - $this->assignView('error_message', t('Requested page could not be found.')); - - $response = $response->withStatus(404); - return $this->respondWithTemplate($response, TemplatePage::ERROR_404); - } -} diff --git a/application/front/controller/visitor/ShaarliVisitorController.php b/application/front/controller/visitor/ShaarliVisitorController.php index 2c91cc3d0..e127bf75c 100644 --- a/application/front/controller/visitor/ShaarliVisitorController.php +++ b/application/front/controller/visitor/ShaarliVisitorController.php @@ -7,7 +7,7 @@ use DI\Container; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Shaarli\Bookmark\BookmarkFilter; +use Shaarli\Front\Controller\PageTrait; /** * Class ShaarliVisitorController @@ -19,6 +19,8 @@ */ abstract class ShaarliVisitorController { + use PageTrait; + /** @var Container */ protected $container; @@ -28,18 +30,6 @@ public function __construct(Container $container) $this->container = $container; } - /** - * Assign variables to RainTPL template through the PageBuilder. - * - * @param mixed $value Value to assign to the template - */ - protected function assignView(string $name, $value): self - { - $this->container->get('pageBuilder')->assign($name, $value); - - return $this; - } - /** * Assign variables to RainTPL template through the PageBuilder. * @@ -54,48 +44,6 @@ protected function assignAllView(array $data): self return $this; } - protected function render(string $template): string - { - // Legacy key that used to be injected by PluginManager - $this->assignView('_PAGE_', $template); - $this->assignView('template', $template); - - $this->assignView('linkcount', $this->container->get('bookmarkService')->count(BookmarkFilter::$ALL)); - $this->assignView('privateLinkcount', $this->container->get('bookmarkService') - ->count(BookmarkFilter::$PRIVATE)); - - $this->executeDefaultHooks($template); - - $this->assignView('plugin_errors', $this->container->get('pluginManager')->getErrors()); - - return $this->container->get('pageBuilder')->render($template, $this->container->get('basePath')); - } - - /** - * Call plugin hooks for header, footer and includes, specifying which page will be rendered. - * Then assign generated data to RainTPL. - */ - protected function executeDefaultHooks(string $template): void - { - $common_hooks = [ - 'includes', - 'header', - 'footer', - ]; - - $parameters = $this->buildPluginParameters($template); - - foreach ($common_hooks as $name) { - $pluginData = []; - $this->container->get('pluginManager')->executeHooks( - 'render_' . $name, - $pluginData, - $parameters - ); - $this->assignView('plugins_' . $name, $pluginData); - } - } - protected function executePageHooks(string $hook, array &$data, string $template = null): void { $this->container->get('pluginManager')->executeHooks( @@ -105,17 +53,6 @@ protected function executePageHooks(string $hook, array &$data, string $template ); } - protected function buildPluginParameters(?string $template): array - { - return [ - 'target' => $template, - 'loggedin' => $this->container->get('loginManager')->isLoggedIn(), - 'basePath' => $this->container->get('basePath'), - 'rootPath' => preg_replace('#/index\.php$#', '', $this->container->get('basePath')), - 'bookmarkService' => $this->container->get('bookmarkService') - ]; - } - /** * Simple helper which prepend the base path to redirect path. * diff --git a/application/helper/DailyPageHelper.php b/application/helper/DailyPageHelper.php index 80dbbe508..8be652464 100644 --- a/application/helper/DailyPageHelper.php +++ b/application/helper/DailyPageHelper.php @@ -21,7 +21,7 @@ class DailyPageHelper * * @param Request $request HTTP request * - * @return string|null month/week/day + * @return string month/week/day */ public static function extractRequestedType(Request $request): string { diff --git a/composer.json b/composer.json index ad75bcf31..b8cd69878 100644 --- a/composer.json +++ b/composer.json @@ -58,6 +58,7 @@ "Shaarli\\Feed\\": "application/feed", "Shaarli\\Formatter\\": "application/formatter", "Shaarli\\Front\\": "application/front", + "Shaarli\\Front\\Controller\\": "application/front/controller", "Shaarli\\Front\\Controller\\Admin\\": "application/front/controller/admin", "Shaarli\\Front\\Controller\\Visitor\\": "application/front/controller/visitor", "Shaarli\\Front\\Exception\\": "application/front/exceptions", diff --git a/index.php b/index.php index 13c6b7f23..ea3c42d3b 100644 --- a/index.php +++ b/index.php @@ -27,8 +27,6 @@ require_once __DIR__ . '/init.php'; use Katzgrau\KLogger\Logger; -use Psr\Http\Message\ResponseInterface as Response; -use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LogLevel; use Shaarli\Api\Controllers as ApiControllers; use Shaarli\Config\ConfigManager; @@ -49,20 +47,14 @@ // Manually override root URL for complex server configurations define('SHAARLI_ROOT_URL', $conf->get('general.root_url', null)); -// In dev mode, throw exception on any warning -$displayErrorDetails = $conf->get('dev.debug', false); // // See all errors (for debugging only) +$displayErrorDetails = $conf->get('dev.debug', false); // In dev mode, throw exception on any warning -if ($conf->get('dev.debug', false)) { +if ($displayErrorDetails) { // See all errors (for debugging only) error_reporting(-1); set_error_handler(function ($errno, $errstr, $errfile, $errline, array $errcontext = []) { - // Skip PHP 8 deprecation warning with Pimple. - if (strpos($errfile, 'src/Pimple/Container.php') !== -1 && strpos($errstr, 'ArrayAccess::') !== -1) { - return error_log($errstr); - } - throw new ErrorException($errstr, 0, $errno, $errfile, $errline); }); } diff --git a/phpunit.xml b/phpunit.xml index 383595a20..90167eb5a 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,9 +1,14 @@ + + + application + + tests @@ -19,5 +24,4 @@ tests/languages/fr - diff --git a/tests/front/controller/visitor/ErrorControllerTest.php b/tests/front/ShaarliErrorHandlerTest.php similarity index 51% rename from tests/front/controller/visitor/ErrorControllerTest.php rename to tests/front/ShaarliErrorHandlerTest.php index df784c01c..7381d6518 100644 --- a/tests/front/controller/visitor/ErrorControllerTest.php +++ b/tests/front/ShaarliErrorHandlerTest.php @@ -2,26 +2,34 @@ declare(strict_types=1); -namespace Shaarli\Front\Controller\Visitor; +namespace Shaarli\Front; +use Shaarli\Front\Controller\Visitor\FrontControllerMockHelper; use Shaarli\Front\Exception\ShaarliFrontException; use Shaarli\TestCase; use Shaarli\Tests\Utils\FakeRequest; -use Slim\Psr7\Response as SlimResponse; +use Slim\CallableResolver; +use Slim\Exception\HttpNotFoundException; +use Slim\Psr7\Factory\ResponseFactory; use Slim\Psr7\Uri; +use Slim\Routing\RouteContext; -class ErrorControllerTest extends TestCase +class ShaarliErrorHandlerTest extends TestCase { use FrontControllerMockHelper; /** @var ErrorController */ - protected $controller; + protected $errorHandler; public function setUp(): void { $this->createContainer(); - - $this->controller = new ErrorController($this->container); + $this->errorHandler = new ShaarliErrorHandler( + new CallableResolver(), + new ResponseFactory(), + null, + $this->container + ); } /** @@ -33,23 +41,21 @@ public function testDisplayFrontExceptionError(): void 'POST', new Uri('', '') ))->withServerParams(['SERVER_NAME' => 'domain.tld', 'SERVER_PORT' => 80]); - $response = new SlimResponse(); $message = 'error message'; $errorCode = 418; + $exception = new class ($message, $errorCode) extends ShaarliFrontException { + }; + // Save RainTPL assigned variables $assignedVariables = []; $this->assignTemplateVars($assignedVariables); - $result = ($this->controller)( - $request, - $response, - new class ($message, $errorCode) extends ShaarliFrontException { - } - ); + $result = ($this->errorHandler)($request, $exception, false, true, true); static::assertSame($errorCode, $result->getStatusCode()); + static::assertSame('error', (string) $result->getBody()); static::assertSame($message, $assignedVariables['message']); static::assertArrayNotHasKey('stacktrace', $assignedVariables); } @@ -61,7 +67,7 @@ public function testDisplayFrontExceptionError(): void public function testDisplayAnyExceptionErrorNoDebugLoggedIn(): void { $request = new FakeRequest(); - $response = new SlimResponse(); + $exception = new \Exception('abc'); // Save RainTPL assigned variables $assignedVariables = []; @@ -69,9 +75,10 @@ public function testDisplayAnyExceptionErrorNoDebugLoggedIn(): void $this->container->get('loginManager')->method('isLoggedIn')->willReturn(true); - $result = ($this->controller)($request, $response, new \Exception('abc')); + $result = ($this->errorHandler)($request, $exception, false, true, true); static::assertSame(500, $result->getStatusCode()); + static::assertSame('error', (string) $result->getBody()); static::assertSame('Error: abc', $assignedVariables['message']); static::assertContainsPolyfill('Please report it on Github', $assignedVariables['text']); static::assertArrayHasKey('stacktrace', $assignedVariables); @@ -84,7 +91,7 @@ public function testDisplayAnyExceptionErrorNoDebugLoggedIn(): void public function testDisplayAnyExceptionErrorNoDebug(): void { $request = new FakeRequest(); - $response = new SlimResponse(); + $exception = new \Exception('abc'); // Save RainTPL assigned variables $assignedVariables = []; @@ -92,11 +99,50 @@ public function testDisplayAnyExceptionErrorNoDebug(): void $this->container->get('loginManager')->method('isLoggedIn')->willReturn(false); - $result = ($this->controller)($request, $response, new \Exception('abc')); + $result = ($this->errorHandler)($request, $exception, false, true, true); static::assertSame(500, $result->getStatusCode()); + static::assertSame('error', (string) $result->getBody()); static::assertSame('An unexpected error occurred.', $assignedVariables['message']); static::assertArrayNotHasKey('text', $assignedVariables); static::assertArrayNotHasKey('stacktrace', $assignedVariables); } + + /** + * Test displaying 404 error + */ + public function testDisplayNotFoundError(): void + { + $request = (new FakeRequest())->withAttribute(RouteContext::BASE_PATH, '/subfolder'); + $exception = new HttpNotFoundException($request); + + // Save RainTPL assigned variables + $assignedVariables = []; + $this->assignTemplateVars($assignedVariables); + + $result = ($this->errorHandler)($request, $exception, false, true, true); + + static::assertSame(404, $result->getStatusCode()); + static::assertSame('404', (string) $result->getBody()); + static::assertSame('Requested page could not be found.', $assignedVariables['error_message']); + } + + /** + * Test displaying 404 error from REST API + */ + public function testDisplayNotFoundErrorFromAPI(): void + { + $request = (new FakeRequest())->withAttribute(RouteContext::BASE_PATH, '/subfolder'); + $exception = new HttpNotFoundException($request); + + // Save RainTPL assigned variables + $assignedVariables = []; + $this->assignTemplateVars($assignedVariables); + + $result = ($this->errorHandler)($request, $exception, false, true, true); + + static::assertSame(404, $result->getStatusCode()); + // next line does not work after Slim4 migration + // static::assertSame([], $assignedVariables); + } } diff --git a/tests/front/controller/visitor/ErrorNotFoundControllerTest.php b/tests/front/controller/visitor/ErrorNotFoundControllerTest.php deleted file mode 100644 index c862bb065..000000000 --- a/tests/front/controller/visitor/ErrorNotFoundControllerTest.php +++ /dev/null @@ -1,68 +0,0 @@ -createContainer(); - - $this->controller = new ErrorNotFoundController($this->container); - } - - /** - * Test displaying 404 error - */ - public function testDisplayNotFoundError(): void - { - $request = (new FakeRequest())->withAttribute(RouteContext::BASE_PATH, '/subfolder'); - - $response = new SlimResponse(); - - // Save RainTPL assigned variables - $assignedVariables = []; - $this->assignTemplateVars($assignedVariables); - - $result = ($this->controller)( - $request, - $response - ); - - static::assertSame(404, $result->getStatusCode()); - static::assertSame('404', (string) $result->getBody()); - static::assertSame('Requested page could not be found.', $assignedVariables['error_message']); - } - - /** - * Test displaying 404 error from REST API - */ - public function testDisplayNotFoundErrorFromAPI(): void - { - $request = (new FakeRequest())->withAttribute(RouteContext::BASE_PATH, '/subfolder'); - - $response = new SlimResponse(); - - // Save RainTPL assigned variables - $assignedVariables = []; - $this->assignTemplateVars($assignedVariables); - - $result = ($this->controller)($request, $response); - - static::assertSame(404, $result->getStatusCode()); - // next line does not work after Slim4 migration - // static::assertSame([], $assignedVariables); - } -} diff --git a/tests/languages/en/UtilsEnTest.php b/tests/languages/en/UtilsEnTest.php index f1e21b9a3..a59f989b5 100644 --- a/tests/languages/en/UtilsEnTest.php +++ b/tests/languages/en/UtilsEnTest.php @@ -23,7 +23,9 @@ public function testDateFormat() $current = setlocale(LC_ALL, 0); autoLocale('en_US.UTF-8'); $date = DateTime::createFromFormat('Ymd_His', '20170102_201112'); - $this->assertRegExp('/January 2, 2017 (at )?8:11:12 PM GMT\+0?3(:00)?/', format_date($date, true, true)); + // Replace No-Break-Space with simple space + $formatedDate = str_replace(' ', ' ', format_date($date, true, true)); + $this->assertRegExp('/January 2, 2017 (at )?8:11:12 PM GMT\+0?3(:00)?/', $formatedDate); setlocale(LC_ALL, $current); }