From c2c9c547d40e61e309d05fe2527ef91334dd4dd4 Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 12:55:09 +0100 Subject: [PATCH 01/12] NanoApp::handleException: handle TypeError and other throwable built-in errors --- classes/NanoApp.class.php | 8 ++++---- tests/app/tests/AppCoreTest.php | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/classes/NanoApp.class.php b/classes/NanoApp.class.php index 3f7561fb..ec9a3a38 100644 --- a/classes/NanoApp.class.php +++ b/classes/NanoApp.class.php @@ -329,14 +329,14 @@ public function render($controllerName, $methodName = '', array $params = []) * Will render HTTP 500 page with error details * * @param callable $fn function to call - * @param callable|bool $handler custom exception handling function to call - * @return mixed|Exception value return by function called or exception that was thrown + * @param callable|null $handler custom exception handling function to call + * @return mixed|Throwable value return by function called or exception that was thrown */ - public function handleException(callable $fn, $handler = false) + public function handleException(callable $fn, ?callable $handler = null) { try { return $fn(); - } catch (\Exception $e) { + } catch (\Throwable $e) { $response = $this->getResponse(); $response->setResponseCode(Response::INTERNAL_SERVER_ERROR); $response->setContentType('text/plain'); diff --git a/tests/app/tests/AppCoreTest.php b/tests/app/tests/AppCoreTest.php index 881fa950..2685b7b5 100644 --- a/tests/app/tests/AppCoreTest.php +++ b/tests/app/tests/AppCoreTest.php @@ -2,6 +2,8 @@ namespace Nano\AppTests; +use Nano\TestApp\TestModel; + /** * Set of unit tests for Nano's Application core */ @@ -75,4 +77,38 @@ public function testAppFactory() // test creation of not existing class $this->assertNull($this->app->factory('NotExistingClass')); } + + /** + * @covers NanoApp::handleException + * @dataProvider handleExceptionDataProvider + */ + public function testHandleException(callable $fn, string $expectedClass) + { + $ret = $this->app->handleException($fn); + $this->assertInstanceOf($expectedClass, $ret); + } + + public static function handleExceptionDataProvider(): \Generator + { + yield 'TypeError' => [ + function () { + array_filter(null); // passing null here triggers the TypeError + }, + \TypeError::class, + ]; + + yield 'Exception' => [ + function () { + throw new \Exception('foo'); + }, + \Exception::class, + ]; + + yield 'Callable returned value is returned by handleException' => [ + function () { + return new TestModel(); + }, + TestModel::class, + ]; + } } From b0014590e428036bce7861db3512d8fbca66c7c0 Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 13:01:57 +0100 Subject: [PATCH 02/12] AppCoreTest: import classes instead of hardcoding their names as strings --- tests/app/tests/AppCoreTest.php | 47 ++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/tests/app/tests/AppCoreTest.php b/tests/app/tests/AppCoreTest.php index 2685b7b5..bc84b831 100644 --- a/tests/app/tests/AppCoreTest.php +++ b/tests/app/tests/AppCoreTest.php @@ -2,7 +2,18 @@ namespace Nano\AppTests; +use Database; +use ExampleModel; +use Nano\Cache; +use Nano\Config; +use Nano\Debug; +use Nano\Events; +use Nano\Request; +use Nano\Response; +use Nano\Router; use Nano\TestApp\TestModel; +use NanoApp; +use NanoCliApp; /** * Set of unit tests for Nano's Application core @@ -11,15 +22,15 @@ class AppCoreTest extends AppTestBase { public function testCreateApp() { - $this->assertInstanceOf('NanoApp', $this->app); - $this->assertInstanceOf('Nano\Cache', $this->app->getCache()); - $this->assertInstanceOf('Nano\Config', $this->app->getConfig()); - $this->assertInstanceOf('Database', $this->app->getDatabase()); - $this->assertInstanceOf('Nano\Debug', $this->app->getDebug()); - $this->assertInstanceOf('Nano\Events', $this->app->getEvents()); - $this->assertInstanceOf('Nano\Request', $this->app->getRequest()); - $this->assertInstanceOf('Nano\Response', $this->app->getResponse()); - $this->assertInstanceOf('Nano\Router', $this->app->getRouter()); + $this->assertInstanceOf(NanoApp::class, $this->app); + $this->assertInstanceOf(Cache::class, $this->app->getCache()); + $this->assertInstanceOf(Config::class, $this->app->getConfig()); + $this->assertInstanceOf(Database::class, $this->app->getDatabase()); + $this->assertInstanceOf(Debug::class, $this->app->getDebug()); + $this->assertInstanceOf(Events::class, $this->app->getEvents()); + $this->assertInstanceOf(Request::class, $this->app->getRequest()); + $this->assertInstanceOf(Response::class, $this->app->getResponse()); + $this->assertInstanceOf(Router::class, $this->app->getRouter()); // directories $this->assertEquals($this->dir, $this->app->getDirectory()); @@ -39,7 +50,7 @@ public function testCliApp() { $app = \Nano::cli($this->dir); - $this->assertInstanceOf('NanoCliApp', $app); + $this->assertInstanceOf(NanoCliApp::class, $app); $this->assertEquals($this->dir . '/log/script.log', $app->getDebug()->getLogFile()); $app = \Nano::cli($this->dir, 'foo'); @@ -50,10 +61,10 @@ public function testCliApp() public function testAppFactory() { // "simple" factory call (no extra params) - $obj = $this->app->factory('ExampleModel'); + $obj = $this->app->factory(ExampleModel::class); - $this->assertInstanceOf('ExampleModel', $obj); - $this->assertInstanceOf('NanoApp', $obj->app); + $this->assertInstanceOf(ExampleModel::class, $obj); + $this->assertInstanceOf(NanoApp::class, $obj->app); $this->assertNull($obj->foo); $this->assertNull($obj->bar); @@ -61,15 +72,15 @@ public function testAppFactory() $this->assertEquals(123, $obj->bar); // "complex" factory call (with extra params) - $obj = $this->app->factory('ExampleModel', ['test']); + $obj = $this->app->factory(ExampleModel::class, ['test']); - $this->assertInstanceOf('ExampleModel', $obj); - $this->assertInstanceOf('NanoApp', $obj->app); + $this->assertInstanceOf(ExampleModel::class, $obj); + $this->assertInstanceOf(NanoApp::class, $obj->app); $this->assertEquals('test', $obj->foo); $this->assertNull($obj->bar); // "complex" factory call (with extra params) - $obj = $this->app->factory('ExampleModel', ['test', 123]); + $obj = $this->app->factory(ExampleModel::class, ['test', 123]); $this->assertEquals('test', $obj->foo); $this->assertEquals(123, $obj->bar); @@ -104,7 +115,7 @@ function () { \Exception::class, ]; - yield 'Callable returned value is returned by handleException' => [ + yield 'Callable return value is returned by handleException' => [ function () { return new TestModel(); }, From f519b2dc053d87d66899570cda6de3b14dfe8a1a Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 13:04:53 +0100 Subject: [PATCH 03/12] NanoBaseTest: improve the typing --- classes/NanoBaseTest.class.php | 3 +-- tests/app/tests/AppTestBase.php | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/classes/NanoBaseTest.class.php b/classes/NanoBaseTest.class.php index ff87cf85..731508a1 100644 --- a/classes/NanoBaseTest.class.php +++ b/classes/NanoBaseTest.class.php @@ -12,8 +12,7 @@ */ class NanoBaseTest extends TestCase { - /* @var $app \NanoApp */ - protected $app; + protected \NanoApp $app; /* @see https://github.com/postmanlabs/httpbin */ const HTTPBIN_HOST = 'http://0.0.0.0:5555'; diff --git a/tests/app/tests/AppTestBase.php b/tests/app/tests/AppTestBase.php index 35ca7c02..547cecf3 100644 --- a/tests/app/tests/AppTestBase.php +++ b/tests/app/tests/AppTestBase.php @@ -10,10 +10,8 @@ */ abstract class AppTestBase extends NanoBaseTest { - /* @var \NanoApp $app */ - protected $app; - protected $dir; - protected $ip; + protected string $dir; + protected string $ip; public function setUp(): void { From bef57b9158634ce9ceacaba3f96ef1885ae5c603 Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 13:18:53 +0100 Subject: [PATCH 04/12] Cover NanoCliApp::handleException too --- classes/NanoCliApp.class.php | 24 ++++++++++++++ tests/app/tests/NanoCliAppTest.php | 52 ++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 tests/app/tests/NanoCliAppTest.php diff --git a/classes/NanoCliApp.class.php b/classes/NanoCliApp.class.php index 47444ce3..93708f1b 100644 --- a/classes/NanoCliApp.class.php +++ b/classes/NanoCliApp.class.php @@ -26,4 +26,28 @@ public function __construct($dir, $configSet = 'default', $logFile = 'script') require $bootstrapFile; } } + + /** + * Wrap your script logic with the method below to exceptions logging added. + */ + public function handleException(callable $fn, ?callable $handler = null) + { + try { + return $fn(); + } catch (\Throwable $ex) { + // log the exception + $logger = NanoLogger::getLogger('nano.app.exception'); + $logger->error($ex->getMessage(), [ + 'exception' => $ex, + ]); + + if (is_callable($handler)) { + $handler($ex); + return $ex; + } + + echo get_class($ex) . ": {$ex->getMessage()}\n{$ex->getTraceAsString()}\n"; + die(1); + } + } } diff --git a/tests/app/tests/NanoCliAppTest.php b/tests/app/tests/NanoCliAppTest.php new file mode 100644 index 00000000..8366ff72 --- /dev/null +++ b/tests/app/tests/NanoCliAppTest.php @@ -0,0 +1,52 @@ +cliApp = \Nano::cli($dir); + } + + /** + * @covers NanoCliApp::handleException + * @dataProvider handleExceptionDataProvider + */ + public function testHandleException(callable $fn, string $expectedClass) + { + // by passing the handler we make handleException() return the exception instead of die() + $ret = $this->cliApp->handleException($fn, handler: function (\Throwable $ex) {}); + $this->assertInstanceOf($expectedClass, $ret); + } + + public static function handleExceptionDataProvider(): \Generator + { + yield 'TypeError' => [ + function () { + array_filter(null); // passing null here triggers the TypeError + }, + \TypeError::class, + ]; + + yield 'Exception' => [ + function () { + throw new \Exception('foo'); + }, + \Exception::class, + ]; + + yield 'Callable return value is returned by handleException' => [ + function () { + return new TestModel(); + }, + TestModel::class, + ]; + } +} From 13c5f5d54aa10643de29319e87f908d1fc69949e Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 16:05:51 +0100 Subject: [PATCH 05/12] AppCoreTest: add @covers annotations --- tests/app/tests/AppCoreTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/app/tests/AppCoreTest.php b/tests/app/tests/AppCoreTest.php index bc84b831..755fdc05 100644 --- a/tests/app/tests/AppCoreTest.php +++ b/tests/app/tests/AppCoreTest.php @@ -20,6 +20,9 @@ */ class AppCoreTest extends AppTestBase { + /** + * @covers NanoApp + */ public function testCreateApp() { $this->assertInstanceOf(NanoApp::class, $this->app); @@ -36,6 +39,9 @@ public function testCreateApp() $this->assertEquals($this->dir, $this->app->getDirectory()); } + /** + * @covers NanoApp::isInAppDirectory + */ public function testIsInAppDirectory() { $this->assertTrue($this->app->isInAppDirectory($this->dir)); @@ -46,6 +52,9 @@ public function testIsInAppDirectory() $this->assertFalse($this->app->isInAppDirectory($this->dir . '/../..')); } + /** + * @covers Nano::cli + */ public function testCliApp() { $app = \Nano::cli($this->dir); @@ -58,6 +67,9 @@ public function testCliApp() $this->assertEquals($this->dir . '/log/foo.log', $app->getDebug()->getLogFile()); } + /** + * @covers NanoApp::factory + */ public function testAppFactory() { // "simple" factory call (no extra params) From 3e34658495d94653b39ae164707fde74e3c16a64 Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 16:06:10 +0100 Subject: [PATCH 06/12] NanoApp: more strict typing --- classes/NanoApp.class.php | 77 ++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 41 deletions(-) diff --git a/classes/NanoApp.class.php b/classes/NanoApp.class.php index ec9a3a38..8a339687 100644 --- a/classes/NanoApp.class.php +++ b/classes/NanoApp.class.php @@ -20,40 +20,37 @@ class NanoApp { // cache object - protected $cache = false; + protected ?Cache $cache = null; // config - protected $config; + protected Config $config; // debug logging - protected $debug; + protected Debug $debug; // database connection - protected $database = false; + protected ?Database $database = null; // events handler - protected $events; + protected Events $events; // response - protected $response; + protected Response $response; // HTTP request - protected $request; + protected Request $request; // router - protected $router; + protected Router $router; // skin - protected $skin = false; - - // an array of loaded modules - protected $modules; + protected ?Skin $skin = null; // application's working directory - protected $dir = ''; + protected string $dir = ''; // current application instance - protected static $app; + protected static ?NanoApp $app = null; /** * Return the current instance of NanoApp @@ -367,19 +364,18 @@ public function handleException(callable $fn, ?callable $handler = null) /** * Return path to application */ - public function getDirectory() + public function getDirectory(): string { return $this->dir; } - /** - * Return cache - * - * @return Cache - */ - public function getCache() + /** + * Lazy-load the cache instance + * @throws Exception + */ + public function getCache(): Cache { - if ($this->cache === false) { + if (is_null($this->cache)) { // setup cache (using default driver if none provided) $cacheSettings = $this->config->get('cache', [ 'driver' => 'file', @@ -394,20 +390,18 @@ public function getCache() /** * Return config */ - public function getConfig() + public function getConfig(): Config { return $this->config; } /** - * Return database - * - * @return Database + * Lazy-load the database instance */ - public function getDatabase() + public function getDatabase(): Database { // lazy connection handling - if ($this->database === false) { + if (is_null($this->database)) { // set connection to database (using db.default config entry) $this->database = Database::connect($this); } @@ -418,7 +412,7 @@ public function getDatabase() /** * Return debug */ - public function getDebug() + public function getDebug(): Debug { return $this->debug; } @@ -426,7 +420,7 @@ public function getDebug() /** * Return events */ - public function getEvents() + public function getEvents(): Events { return $this->events; } @@ -434,7 +428,7 @@ public function getEvents() /** * Return response */ - public function getResponse() + public function getResponse(): Response { return $this->response; } @@ -442,7 +436,7 @@ public function getResponse() /** * Return request */ - public function getRequest() + public function getRequest(): Request { return $this->request; } @@ -451,7 +445,7 @@ public function getRequest() * Set a request for this NanoApp, used by tests * @param Request $request */ - public function setRequest(Request $request) + public function setRequest(Request $request): void { $this->request = $request; } @@ -459,20 +453,21 @@ public function setRequest(Request $request) /** * Return router */ - public function getRouter() + public function getRouter(): Router { return $this->router; } - /** - * Return skin - * - * @return Skin - */ - public function getSkin() + /** + * Return skin + * + * @return Skin + * @throws Exception + */ + public function getSkin(): Skin { // lazy load the skin - if ($this->skin === false) { + if (is_null($this->skin)) { // use the default skin $skinName = $this->config->get('skin', 'default'); From 9898bb722d8e1d612caf1b9f76763cd190db7a3a Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 16:06:32 +0100 Subject: [PATCH 07/12] NanoApp: reformat the code --- classes/NanoApp.class.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/classes/NanoApp.class.php b/classes/NanoApp.class.php index 8a339687..b3504311 100644 --- a/classes/NanoApp.class.php +++ b/classes/NanoApp.class.php @@ -369,10 +369,10 @@ public function getDirectory(): string return $this->dir; } - /** - * Lazy-load the cache instance - * @throws Exception - */ + /** + * Lazy-load the cache instance + * @throws Exception + */ public function getCache(): Cache { if (is_null($this->cache)) { @@ -458,12 +458,12 @@ public function getRouter(): Router return $this->router; } - /** - * Return skin - * - * @return Skin - * @throws Exception - */ + /** + * Return skin + * + * @return Skin + * @throws Exception + */ public function getSkin(): Skin { // lazy load the skin From e2b6f4bcb460d02dad8796a55b75c9d4549693e9 Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 16:13:19 +0100 Subject: [PATCH 08/12] Add SkinTestApp class --- tests/app/classes/SkinTestApp.class.php | 5 +++++ tests/app/config/default.config.php | 1 + 2 files changed, 6 insertions(+) create mode 100644 tests/app/classes/SkinTestApp.class.php diff --git a/tests/app/classes/SkinTestApp.class.php b/tests/app/classes/SkinTestApp.class.php new file mode 100644 index 00000000..1f173972 --- /dev/null +++ b/tests/app/classes/SkinTestApp.class.php @@ -0,0 +1,5 @@ + 'file', ]; From 21ab0efe95013d1d6e883a364b3c32b155fdd086 Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 16:13:32 +0100 Subject: [PATCH 09/12] NanoApp: cast the skin name to a string --- classes/NanoApp.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/NanoApp.class.php b/classes/NanoApp.class.php index b3504311..11bb8a2d 100644 --- a/classes/NanoApp.class.php +++ b/classes/NanoApp.class.php @@ -469,7 +469,7 @@ public function getSkin(): Skin // lazy load the skin if (is_null($this->skin)) { // use the default skin - $skinName = $this->config->get('skin', 'default'); + $skinName = (string) $this->config->get('skin', 'default'); // allow to override the default choice $this->events->fire('NanoAppGetSkin', [$this, &$skinName]); From 977962d5984f3c6872b24d28ab0f3826ef885148 Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 16:13:46 +0100 Subject: [PATCH 10/12] AppCoreTest: cover NanoApp::getSkin() --- tests/app/tests/AppCoreTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/app/tests/AppCoreTest.php b/tests/app/tests/AppCoreTest.php index 755fdc05..d9b7bdc2 100644 --- a/tests/app/tests/AppCoreTest.php +++ b/tests/app/tests/AppCoreTest.php @@ -34,6 +34,7 @@ public function testCreateApp() $this->assertInstanceOf(Request::class, $this->app->getRequest()); $this->assertInstanceOf(Response::class, $this->app->getResponse()); $this->assertInstanceOf(Router::class, $this->app->getRouter()); + $this->assertInstanceOf(\SkinTestApp::class, $this->app->getSkin()); // directories $this->assertEquals($this->dir, $this->app->getDirectory()); From 8ebce4c740d59211d9af8afa8770acc910a2b1e3 Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 16:19:04 +0100 Subject: [PATCH 11/12] NanoObject: lazy-load databases --- classes/NanoApp.class.php | 5 +++++ classes/NanoObject.class.php | 12 ++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/classes/NanoApp.class.php b/classes/NanoApp.class.php index 11bb8a2d..cd6a81ac 100644 --- a/classes/NanoApp.class.php +++ b/classes/NanoApp.class.php @@ -397,6 +397,7 @@ public function getConfig(): Config /** * Lazy-load the database instance + * @throws Exception */ public function getDatabase(): Database { @@ -404,6 +405,10 @@ public function getDatabase(): Database if (is_null($this->database)) { // set connection to database (using db.default config entry) $this->database = Database::connect($this); + + if (is_null($this->database)) { + throw new Exception(__METHOD__ . ': unable to create a database instance!'); + } } return $this->database; diff --git a/classes/NanoObject.class.php b/classes/NanoObject.class.php index 8044fb3c..de7c76ee 100644 --- a/classes/NanoObject.class.php +++ b/classes/NanoObject.class.php @@ -21,9 +21,6 @@ abstract class NanoObject protected $logger; - // DB connection - protected $database; - // config protected $config; @@ -42,15 +39,14 @@ public function __construct() $this->debug = $this->app->getDebug(); $this->logger = NanoLogger::getLogger('nano.' . get_class($this)); $this->events = $this->app->getEvents(); - - // use lazy-resolving - $this->database = static::getDatabase($this->app); } - + /** * Allow models and services to use different database + * + * @throws \Exception */ - protected static function getDatabase(\NanoApp $app) + protected static function getDatabase(\NanoApp $app): \Database { return $app->getDatabase(); } From b10974aeb700be54a2ea4acf98e650e09fbef879 Mon Sep 17 00:00:00 2001 From: macbre Date: Tue, 9 May 2023 16:22:30 +0100 Subject: [PATCH 12/12] HealthcheckBase: no longer need to overwrite the getDatabase() method --- classes/healtcheck/HealtcheckBase.php | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/classes/healtcheck/HealtcheckBase.php b/classes/healtcheck/HealtcheckBase.php index bc52b9b0..7cc4e04a 100644 --- a/classes/healtcheck/HealtcheckBase.php +++ b/classes/healtcheck/HealtcheckBase.php @@ -28,17 +28,6 @@ public function __construct() $this->response->setContentType('text/plain'); } - /** - * Do not connect to the database be default! - * - * @param \NanoApp $app - * @return \Database|null - */ - protected static function getDatabase(\NanoApp $app) - { - return null; - } - /** * @param string $msg */