From 0b0fd299134640d2e97984918f87ec91424dc4b8 Mon Sep 17 00:00:00 2001 From: Greg Sarjeant Date: Sat, 2 Aug 2025 19:52:33 +0000 Subject: [PATCH] simplify-dependency-injection (#44) Closes https://gitea.subcultureofone.org/greg/tkr/issues/43 Use a global $app dictionary to manage global state rather than having complex class constructors that expect three input arguments. Update and fix tests. Add tests for Util class functions that broke in the refactor. Reviewed-on: https://gitea.subcultureofone.org/greg/tkr/pulls/44 Co-authored-by: Greg Sarjeant Co-committed-by: Greg Sarjeant --- public/index.php | 27 ++-- .../AdminController/AdminController.php | 44 +++--- .../AuthController/AuthController.php | 16 ++- src/Controller/Controller.php | 2 - .../CssController/CssController.php | 24 ++-- .../EmojiController/EmojiController.php | 9 +- .../FeedController/FeedController.php | 16 ++- .../HomeController/HomeController.php | 20 ++- .../LogController/LogController.php | 12 +- .../MoodController/MoodController.php | 20 ++- .../TickController/TickController.php | 4 +- src/Framework/Log/Log.php | 4 +- src/Framework/Router/Router.php | 4 +- src/Framework/Util/Util.php | 22 +-- .../AdminController/AdminControllerTest.php | 74 ++++++---- .../FeedController/FeedControllerTest.php | 26 ++-- .../HomeController/HomeControllerTest.php | 40 +++--- .../LogController/LogControllerTest.php | 66 ++++----- tests/Framework/Log/LogTest.php | 45 +++--- tests/Framework/Util/UtilTest.php | 130 ++++++++++++++++++ 20 files changed, 389 insertions(+), 216 deletions(-) diff --git a/public/index.php b/public/index.php index 2fa0a57..8f7f214 100644 --- a/public/index.php +++ b/public/index.php @@ -41,21 +41,14 @@ if (!(preg_match('/setup$/', $path))) { } } -// Get a database connection -// TODO: Change from static function. -global $db; +// Initialize application context with all dependencies +global $app; $db = Database::get(); - -// Initialize core entities -// Defining these as globals isn't great practice, -// but this is a small, single-user app and this data will rarely change. -global $config; -global $user; - -$config = new ConfigModel($db); -$config = $config->loadFromDatabase(); -$user = new UserModel($db); -$user = $user->loadFromDatabase(); +$app = [ + 'db' => $db, + 'config' => (new ConfigModel($db))->loadFromDatabase(), + 'user' => (new UserModel($db))->loadFromDatabase(), +]; // Start a session and generate a CSRF Token // if there isn't already an active session @@ -63,8 +56,8 @@ Session::start(); Session::generateCsrfToken(); // Remove the base path from the URL -if (strpos($path, $config->basePath) === 0) { - $path = substr($path, strlen($config->basePath)); +if (strpos($path, $app['config']->basePath) === 0) { + $path = substr($path, strlen($app['config']->basePath)); } // strip the trailing slash from the resulting route @@ -99,7 +92,7 @@ if ($method === 'POST' && $path != 'setup') { header('Content-Type: text/html; charset=utf-8'); // Render the requested route or throw a 404 -$router = new Router($db, $config, $user); +$router = new Router(); if (!$router->route($path, $method)){ http_response_code(404); echo "404 - Page Not Found"; diff --git a/src/Controller/AdminController/AdminController.php b/src/Controller/AdminController/AdminController.php index f2ff3a6..afccd5d 100644 --- a/src/Controller/AdminController/AdminController.php +++ b/src/Controller/AdminController/AdminController.php @@ -13,22 +13,26 @@ class AdminController extends Controller { } public function getAdminData(bool $isSetup): array { + global $app; + Log::debug("Loading admin page" . ($isSetup ? " (setup mode)" : "")); return [ - 'user' => $this->user, - 'config' => $this->config, + 'user' => $app['user'], + 'config' => $app['config'], 'isSetup' => $isSetup, ]; } public function handleSave(){ + global $app; + if (!Session::isLoggedIn()){ - header('Location: ' . Util::buildRelativeUrl($this->config->basePath, 'login')); + header('Location: ' . Util::buildRelativeUrl($app['config']->basePath, 'login')); exit; } - $result = $this->processSettingsSave($_POST, false); + $result = $this->saveSettings($_POST, false); header('Location: ' . $_SERVER['PHP_SELF']); exit; } @@ -36,12 +40,14 @@ class AdminController extends Controller { public function handleSetup(){ // for setup, we don't care if they're logged in // (because they can't be until setup is complete) - $result = $this->processSettingsSave($_POST, true); + $result = $this->saveSettings($_POST, true); header('Location: ' . $_SERVER['PHP_SELF']); exit; } - public function processSettingsSave(array $postData, bool $isSetup): array { + public function saveSettings(array $postData, bool $isSetup): array { + global $app; + $result = ['success' => false, 'errors' => []]; Log::debug("Processing settings save" . ($isSetup ? " (setup mode)" : "")); @@ -122,30 +128,30 @@ class AdminController extends Controller { if (empty($errors)) { try { // Update site settings - $this->config->siteTitle = $siteTitle; - $this->config->siteDescription = $siteDescription; - $this->config->baseUrl = $baseUrl; - $this->config->basePath = $basePath; - $this->config->itemsPerPage = $itemsPerPage; - $this->config->strictAccessibility = $strictAccessibility; - $this->config->logLevel = $logLevel; + $app['config']->siteTitle = $siteTitle; + $app['config']->siteDescription = $siteDescription; + $app['config']->baseUrl = $baseUrl; + $app['config']->basePath = $basePath; + $app['config']->itemsPerPage = $itemsPerPage; + $app['config']->strictAccessibility = $strictAccessibility; + $app['config']->logLevel = $logLevel; // Save site settings and reload config from database - $this->config = $this->config->save(); + $app['config'] = $app['config']->save(); Log::info("Site settings updated"); // Update user profile - $this->user->username = $username; - $this->user->displayName = $displayName; - $this->user->website = $website; + $app['user']->username = $username; + $app['user']->displayName = $displayName; + $app['user']->website = $website; // Save user profile and reload user from database - $this->user = $this->user->save(); + $app['user'] = $app['user']->save(); Log::info("User profile updated"); // Update the password if one was sent if($password){ - $this->user->setPassword($password); + $app['user']->setPassword($password); Log::info("User password updated"); } diff --git a/src/Controller/AuthController/AuthController.php b/src/Controller/AuthController/AuthController.php index 1ab8189..c5e5f1f 100644 --- a/src/Controller/AuthController/AuthController.php +++ b/src/Controller/AuthController/AuthController.php @@ -1,11 +1,12 @@ $config, + 'config' => $app['config'], 'csrf_token' => $csrf_token, 'error' => $error, ]; @@ -14,7 +15,7 @@ class AuthController extends Controller { } function handleLogin(){ - global $config; + global $app; if ($_SERVER['REQUEST_METHOD'] === 'POST') { $username = $_POST['username'] ?? ''; @@ -22,7 +23,7 @@ class AuthController extends Controller { Log::debug("Login attempt for user {$username}"); - $userModel = new UserModel(); + $userModel = new UserModel($app['db']); $user = $userModel->getByUsername($username); //if ($user && password_verify($password, $user['password_hash'])) { @@ -30,7 +31,7 @@ class AuthController extends Controller { Log::info("Successful login for {$username}"); Session::newLoginSession($user); - header('Location: ' . Util::buildRelativeUrl($config->basePath)); + header('Location: ' . Util::buildRelativeUrl($app['config']->basePath)); exit; } else { Log::warning("Failed login for {$username}"); @@ -44,11 +45,12 @@ class AuthController extends Controller { } function handleLogout(){ + global $app; + Log::info("Logout from user " . $_SESSION['username']); Session::end(); - global $config; - header('Location: ' . Util::buildRelativeUrl($config->basePath)); + header('Location: ' . Util::buildRelativeUrl($app['config']->basePath)); exit; } } \ No newline at end of file diff --git a/src/Controller/Controller.php b/src/Controller/Controller.php index d72bd58..043acf2 100644 --- a/src/Controller/Controller.php +++ b/src/Controller/Controller.php @@ -1,7 +1,5 @@ $user, - 'config' => $config, + 'user' => $app['user'], + 'config' => $app['config'], 'customCss' => $customCss, ]; @@ -49,8 +49,6 @@ class CssController extends Controller { } public function handlePost() { - global $config; - switch ($_POST['action']) { case 'upload': $this->handleUpload(); @@ -69,7 +67,7 @@ class CssController extends Controller { } public function handleDelete(): void{ - global $config; + global $app; // Don't try to delete the default theme. if (!$_POST['selectCssFile']){ @@ -113,26 +111,26 @@ class CssController extends Controller { } // Set the theme back to default - $config->cssId = null; - $config = $config->save(); + $app['config']->cssId = null; + $app['config'] = $app['config']->save(); // Set flash message Session::setFlashMessage('success', 'Theme ' . $cssFilename . ' deleted.'); } private function handleSetTheme() { - global $config; + global $app; if ($_POST['selectCssFile']){ // Set custom theme - $config->cssId = $_POST['selectCssFile']; + $app['config']->cssId = $_POST['selectCssFile']; } else { // Set default theme - $config->cssId = null; + $app['config']->cssId = null; } // Update the site theme - $config = $config->save(); + $app['config'] = $app['config']->save(); // Set flash message Session::setFlashMessage('success', 'Theme applied.'); diff --git a/src/Controller/EmojiController/EmojiController.php b/src/Controller/EmojiController/EmojiController.php index 07520c9..9989f92 100644 --- a/src/Controller/EmojiController/EmojiController.php +++ b/src/Controller/EmojiController/EmojiController.php @@ -2,11 +2,12 @@ class EmojiController extends Controller { // Shows the custom emoji management page public function index(){ - global $config; + global $app; + $emojiList = EmojiModel::loadAll(); $vars = [ - 'config' => $config, + 'config' => $app['config'], 'emojiList' => $emojiList, ]; @@ -14,7 +15,7 @@ } public function handlePost(): void { - global $config; + global $app; switch ($_POST['action']) { case 'add': @@ -29,7 +30,7 @@ break; } - header('Location: ' . Util::buildRelativeUrl($config->basePath, 'admin/emoji')); + header('Location: ' . Util::buildRelativeUrl($app['config']->basePath, 'admin/emoji')); exit; } diff --git a/src/Controller/FeedController/FeedController.php b/src/Controller/FeedController/FeedController.php index 77eb8ae..b20b687 100644 --- a/src/Controller/FeedController/FeedController.php +++ b/src/Controller/FeedController/FeedController.php @@ -2,17 +2,19 @@ class FeedController extends Controller { private $ticks; - public function __construct(PDO $db, ConfigModel $config, UserModel $user){ - parent::__construct($db, $config, $user); + public function __construct() { + global $app; - $tickModel = new TickModel($db, $config); - $this->ticks = $tickModel->getPage($config->itemsPerPage); + $tickModel = new TickModel($app['db'], $app['config']); + $this->ticks = $tickModel->getPage($app['config']->itemsPerPage); Log::debug("Loaded " . count($this->ticks) . " ticks for feeds"); } public function rss(){ - $generator = new RssGenerator($this->config, $this->ticks); + global $app; + + $generator = new RssGenerator($app['config'], $this->ticks); Log::debug("Generating RSS feed with " . count($this->ticks) . " ticks"); header('Content-Type: ' . $generator->getContentType()); @@ -20,7 +22,9 @@ class FeedController extends Controller { } public function atom(){ - $generator = new AtomGenerator($this->config, $this->ticks); + global $app; + + $generator = new AtomGenerator($app['config'], $this->ticks); Log::debug("Generating Atom feed with " . count($this->ticks) . " ticks"); header('Content-Type: ' . $generator->getContentType()); diff --git a/src/Controller/HomeController/HomeController.php b/src/Controller/HomeController/HomeController.php index 06e5097..820a82b 100644 --- a/src/Controller/HomeController/HomeController.php +++ b/src/Controller/HomeController/HomeController.php @@ -9,21 +9,23 @@ class HomeController extends Controller { } public function getHomeData(int $page): array { + global $app; + Log::debug("Loading home page $page"); - $tickModel = new TickModel($this->db, $this->config); - $limit = $this->config->itemsPerPage; + $tickModel = new TickModel($app['db'], $app['config']); + $limit = $app['config']->itemsPerPage; $offset = ($page - 1) * $limit; $ticks = $tickModel->getPage($limit, $offset); - $view = new TicksView($this->config, $ticks, $page); + $view = new TicksView($app['config'], $ticks, $page); $tickList = $view->getHtml(); Log::info("Home page loaded with " . count($ticks) . " ticks"); return [ - 'config' => $this->config, - 'user' => $this->user, + 'config' => $app['config'], + 'user' => $app['user'], 'tickList' => $tickList, ]; } @@ -31,14 +33,18 @@ class HomeController extends Controller { // POST handler // Saves the tick and reloads the homepage public function handleTick(){ + global $app; + $result = $this->processTick($_POST); // redirect to the index (will show the latest tick if one was sent) - header('Location: ' . Util::buildRelativeUrl($this->config->basePath)); + header('Location: ' . Util::buildRelativeUrl($app['config']->basePath)); exit; } public function processTick(array $postData): array { + global $app; + $result = ['success' => false, 'message' => '']; if (!isset($postData['new_tick'])) { @@ -55,7 +61,7 @@ class HomeController extends Controller { } try { - $tickModel = new TickModel($this->db, $this->config); + $tickModel = new TickModel($app['db'], $app['config']); $tickModel->insert($tickContent); Log::info("New tick created: " . substr($tickContent, 0, 50) . (strlen($tickContent) > 50 ? '...' : '')); $result['success'] = true; diff --git a/src/Controller/LogController/LogController.php b/src/Controller/LogController/LogController.php index f9fe2fb..3bccfd5 100644 --- a/src/Controller/LogController/LogController.php +++ b/src/Controller/LogController/LogController.php @@ -2,16 +2,16 @@ class LogController extends Controller { private string $storageDir; - public function __construct(PDO $db, ConfigModel $config, UserModel $user, ?string $storageDir = null) { - parent::__construct($db, $config, $user); + public function __construct(?string $storageDir = null) { $this->storageDir = $storageDir ?? STORAGE_DIR; } public function index() { + global $app; + // Ensure user is logged in if (!Session::isLoggedIn()) { - global $config; - header('Location: ' . Util::buildRelativeUrl($config->basePath, 'login')); + header('Location: ' . Util::buildRelativeUrl($app['config']->basePath, 'login')); exit; } @@ -26,7 +26,7 @@ class LogController extends Controller { } public function getLogData(string $levelFilter = '', string $routeFilter = ''): array { - global $config; + global $app; $limit = 300; // Show last 300 log entries @@ -38,7 +38,7 @@ class LogController extends Controller { $availableLevels = ['DEBUG', 'INFO', 'WARNING', 'ERROR']; return [ - 'config' => $config, + 'config' => $app['config'], 'logEntries' => $logEntries, 'availableRoutes' => $availableRoutes, 'availableLevels' => $availableLevels, diff --git a/src/Controller/MoodController/MoodController.php b/src/Controller/MoodController/MoodController.php index f586ae2..450df86 100644 --- a/src/Controller/MoodController/MoodController.php +++ b/src/Controller/MoodController/MoodController.php @@ -1,14 +1,14 @@ render_mood_picker(self::getEmojisWithLabels(), $user->mood); + $moodPicker = $view->render_mood_picker(self::getEmojisWithLabels(), $app['user']->mood); $vars = [ - 'config' => $config, + 'config' => $app['config'], 'moodPicker' => $moodPicker, ]; @@ -16,11 +16,9 @@ } public function handlePost(){ + global $app; + if ($_SERVER['REQUEST_METHOD'] === 'POST') { - // Get the data we need - global $config; - global $user; - switch ($_POST['action']){ case 'set': $mood = $_POST['mood']; @@ -31,11 +29,11 @@ } // set or clear the mood - $user->mood = $mood; - $user = $user->save(); + $app['user']->mood = $mood; + $app['user'] = $app['user']->save(); // go back to the index and show the updated mood - header('Location: ' . Util::buildRelativeUrl($config->basePath)); + header('Location: ' . Util::buildRelativeUrl($app['config']->basePath)); exit; } } diff --git a/src/Controller/TickController/TickController.php b/src/Controller/TickController/TickController.php index 7b1f2f3..ab2e887 100644 --- a/src/Controller/TickController/TickController.php +++ b/src/Controller/TickController/TickController.php @@ -3,7 +3,9 @@ class TickController extends Controller{ //public function index(string $year, string $month, string $day, string $hour, string $minute, string $second){ public function index(int $id){ - $tickModel = new TickModel(); + global $app; + + $tickModel = new TickModel($app['db'], $app['config']); $vars = $tickModel->get($id); $this->render('tick.php', $vars); } diff --git a/src/Framework/Log/Log.php b/src/Framework/Log/Log.php index 5c41fdc..08a818b 100644 --- a/src/Framework/Log/Log.php +++ b/src/Framework/Log/Log.php @@ -44,8 +44,8 @@ class Log { } private static function write($level, $message) { - global $config; - $logLevel = $config->logLevel ?? self::LEVELS['INFO']; + global $app; + $logLevel = $app['config']->logLevel ?? self::LEVELS['INFO']; // Only log messages if they're at or above the configured log level. if (self::LEVELS[$level] < $logLevel){ diff --git a/src/Framework/Router/Router.php b/src/Framework/Router/Router.php index 3cea851..28d3dcb 100644 --- a/src/Framework/Router/Router.php +++ b/src/Framework/Router/Router.php @@ -1,8 +1,6 @@ db, $this->config, $this->user); + $instance = new $controllerName(); call_user_func_array([$instance, $functionName], $matches); return true; } diff --git a/src/Framework/Util/Util.php b/src/Framework/Util/Util.php index 9456cb3..2741a42 100644 --- a/src/Framework/Util/Util.php +++ b/src/Framework/Util/Util.php @@ -25,12 +25,12 @@ class Util { return preg_replace_callback( '~(https?://[^\s<>"\'()]+)~i', function($matches) use ($link_attrs) { - global $config; + global $app; $escaped_url = rtrim($matches[1], '.,!?;:)]}>'); $clean_url = html_entity_decode($escaped_url, ENT_QUOTES, 'UTF-8'); - $tabIndex = $config->strictAccessibility ? ' tabindex="0" ' : ' '; + $tabIndex = $app['config']->strictAccessibility ? ' tabindex="0"' : ''; - return '' . $escaped_url . ''; + return '' . $escaped_url . ''; }, $text ); @@ -66,41 +66,41 @@ class Util { public static function buildUrl(string $baseUrl, string $basePath, string $path = ''): string { // Normalize baseUrl (remove trailing slash) $baseUrl = rtrim($baseUrl, '/'); - + // Normalize basePath (ensure leading slash, remove trailing slash unless it's just '/') if ($basePath === '' || $basePath === '/') { $basePath = '/'; } else { $basePath = '/' . trim($basePath, '/') . '/'; } - + // Normalize path (remove leading slash if present) $path = ltrim($path, '/'); - + return $baseUrl . $basePath . $path; } public static function buildRelativeUrl(string $basePath, string $path = ''): string { // Ensure basePath starts with / for relative URLs $basePath = '/' . ltrim($basePath, '/'); - + // Remove trailing slash unless it's just '/' if ($basePath !== '/') { $basePath = rtrim($basePath, '/'); } - + // Add path $path = ltrim($path, '/'); - + if ($path === '') { return $basePath; } - + // If basePath is root, don't add extra slash if ($basePath === '/') { return '/' . $path; } - + return $basePath . '/' . $path; } } \ No newline at end of file diff --git a/tests/Controller/AdminController/AdminControllerTest.php b/tests/Controller/AdminController/AdminControllerTest.php index 1725369..5618419 100644 --- a/tests/Controller/AdminController/AdminControllerTest.php +++ b/tests/Controller/AdminController/AdminControllerTest.php @@ -15,13 +15,8 @@ class AdminControllerTest extends TestCase $this->tempLogDir = sys_get_temp_dir() . '/tkr_test_logs_' . uniqid(); mkdir($this->tempLogDir . '/logs', 0777, true); Log::init($this->tempLogDir . '/logs/tkr.log'); - - // Set up global config for logging level (DEBUG = 1) - global $config; - $config = new stdClass(); - $config->logLevel = 1; // Allow DEBUG level logs - // Create mock PDO (needed for base constructor) + // Create mock PDO $this->mockPdo = $this->createMock(PDO::class); // Create real config and user objects with mocked PDO @@ -36,6 +31,17 @@ class AdminControllerTest extends TestCase $this->user->username = 'testuser'; $this->user->displayName = 'Test User'; $this->user->website = 'https://example.com'; + + // Set up global $app for simplified dependency access + global $app; + $app = [ + 'db' => $this->mockPdo, + 'config' => $this->config, + 'user' => $this->user, + ]; + + // Set log level on config for Log class + $this->config->logLevel = 1; // Allow DEBUG level logs } protected function tearDown(): void @@ -60,7 +66,7 @@ class AdminControllerTest extends TestCase public function testGetAdminDataRegularMode(): void { - $controller = new AdminController($this->mockPdo, $this->config, $this->user); + $controller = new AdminController(); $data = $controller->getAdminData(false); // Should return proper structure @@ -76,7 +82,7 @@ class AdminControllerTest extends TestCase public function testGetAdminDataSetupMode(): void { - $controller = new AdminController($this->mockPdo, $this->config, $this->user); + $controller = new AdminController(); $data = $controller->getAdminData(true); // Should return proper structure @@ -92,8 +98,8 @@ class AdminControllerTest extends TestCase public function testProcessSettingsSaveWithEmptyData(): void { - $controller = new AdminController($this->mockPdo, $this->config, $this->user); - $result = $controller->processSettingsSave([], false); + $controller = new AdminController(); + $result = $controller->saveSettings([], false); $this->assertFalse($result['success']); $this->assertContains('No data provided', $result['errors']); @@ -101,7 +107,7 @@ class AdminControllerTest extends TestCase public function testProcessSettingsSaveValidationErrors(): void { - $controller = new AdminController($this->mockPdo, $this->config, $this->user); + $controller = new AdminController(); // Test data with multiple validation errors $postData = [ @@ -116,7 +122,7 @@ class AdminControllerTest extends TestCase 'confirm_password' => 'different' // Passwords don't match ]; - $result = $controller->processSettingsSave($postData, false); + $result = $controller->saveSettings($postData, false); $this->assertFalse($result['success']); $this->assertNotEmpty($result['errors']); @@ -157,7 +163,12 @@ class AdminControllerTest extends TestCase $config = new ConfigModel($this->mockPdo); $user = new UserModel($this->mockPdo); - $controller = new AdminController($this->mockPdo, $config, $user); + // Update global $app with test models + global $app; + $app['config'] = $config; + $app['user'] = $user; + + $controller = new AdminController(); $postData = [ 'username' => 'newuser', @@ -172,7 +183,7 @@ class AdminControllerTest extends TestCase 'log_level' => 2 ]; - $result = $controller->processSettingsSave($postData, false); + $result = $controller->saveSettings($postData, false); $this->assertTrue($result['success']); $this->assertEmpty($result['errors']); @@ -214,7 +225,12 @@ class AdminControllerTest extends TestCase $config = new ConfigModel($this->mockPdo); $user = new UserModel($this->mockPdo); - $controller = new AdminController($this->mockPdo, $config, $user); + // Update global $app with test models + global $app; + $app['config'] = $config; + $app['user'] = $user; + + $controller = new AdminController(); $postData = [ 'username' => 'testuser', @@ -228,7 +244,7 @@ class AdminControllerTest extends TestCase 'confirm_password' => 'newpassword' ]; - $result = $controller->processSettingsSave($postData, false); + $result = $controller->saveSettings($postData, false); $this->assertTrue($result['success']); } @@ -242,7 +258,12 @@ class AdminControllerTest extends TestCase $config = new ConfigModel($this->mockPdo); $user = new UserModel($this->mockPdo); - $controller = new AdminController($this->mockPdo, $config, $user); + // Update global $app with test models + global $app; + $app['config'] = $config; + $app['user'] = $user; + + $controller = new AdminController(); $postData = [ 'username' => 'testuser', @@ -254,7 +275,7 @@ class AdminControllerTest extends TestCase 'items_per_page' => 10 ]; - $result = $controller->processSettingsSave($postData, false); + $result = $controller->saveSettings($postData, false); $this->assertFalse($result['success']); $this->assertContains('Failed to save settings', $result['errors']); @@ -262,7 +283,7 @@ class AdminControllerTest extends TestCase public function testLoggingOnAdminPageLoad(): void { - $controller = new AdminController($this->mockPdo, $this->config, $this->user); + $controller = new AdminController(); $controller->getAdminData(false); // Check that logs were written @@ -275,7 +296,7 @@ class AdminControllerTest extends TestCase public function testLoggingOnSetupPageLoad(): void { - $controller = new AdminController($this->mockPdo, $this->config, $this->user); + $controller = new AdminController(); $controller->getAdminData(true); // Check that logs were written @@ -288,7 +309,7 @@ class AdminControllerTest extends TestCase public function testLoggingOnValidationErrors(): void { - $controller = new AdminController($this->mockPdo, $this->config, $this->user); + $controller = new AdminController(); $postData = [ 'username' => '', // Will cause validation error @@ -299,7 +320,7 @@ class AdminControllerTest extends TestCase 'items_per_page' => 10 ]; - $controller->processSettingsSave($postData, false); + $controller->saveSettings($postData, false); // Check that logs were written $logFile = $this->tempLogDir . '/logs/tkr.log'; @@ -341,7 +362,12 @@ class AdminControllerTest extends TestCase $config = new ConfigModel($this->mockPdo); $user = new UserModel($this->mockPdo); - $controller = new AdminController($this->mockPdo, $config, $user); + // Update global $app with test models + global $app; + $app['config'] = $config; + $app['user'] = $user; + + $controller = new AdminController(); $postData = [ 'username' => 'testuser', @@ -353,7 +379,7 @@ class AdminControllerTest extends TestCase 'items_per_page' => 10 ]; - $controller->processSettingsSave($postData, false); + $controller->saveSettings($postData, false); // Check that logs were written $logFile = $this->tempLogDir . '/logs/tkr.log'; diff --git a/tests/Controller/FeedController/FeedControllerTest.php b/tests/Controller/FeedController/FeedControllerTest.php index 4f8ddd1..3cfe81f 100644 --- a/tests/Controller/FeedController/FeedControllerTest.php +++ b/tests/Controller/FeedController/FeedControllerTest.php @@ -16,11 +16,6 @@ class FeedControllerTest extends TestCase mkdir($this->tempLogDir . '/logs', 0777, true); Log::init($this->tempLogDir . '/logs/tkr.log'); - // Set up global config for logging level (DEBUG = 1) - global $config; - $config = new stdClass(); - $config->logLevel = 1; // Allow DEBUG level logs - // Create mock PDO and PDOStatement $this->mockStatement = $this->createMock(PDOStatement::class); $this->mockPdo = $this->createMock(PDO::class); @@ -36,6 +31,17 @@ class FeedControllerTest extends TestCase // Mock user $this->mockUser = new UserModel($this->mockPdo); $this->mockUser->displayName = 'Test User'; + + // Set up global $app for simplified dependency access + global $app; + $app = [ + 'db' => $this->mockPdo, + 'config' => $this->mockConfig, + 'user' => $this->mockUser, + ]; + + // Set log level on config for Log class + $this->mockConfig->logLevel = 1; // Allow DEBUG level logs } protected function tearDown(): void @@ -77,7 +83,7 @@ class FeedControllerTest extends TestCase { $this->setupMockDatabase([]); - $controller = new FeedController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new FeedController(); // Verify it was created successfully $this->assertInstanceOf(FeedController::class, $controller); @@ -99,7 +105,7 @@ class FeedControllerTest extends TestCase $this->setupMockDatabase($testTicks); - $controller = new FeedController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new FeedController(); // Verify it was created successfully $this->assertInstanceOf(FeedController::class, $controller); @@ -127,7 +133,7 @@ class FeedControllerTest extends TestCase ->method('execute') ->with([10, 0]); // itemsPerPage=10, page 1 = offset 0 - new FeedController($this->mockPdo, $this->mockConfig, $this->mockUser); + new FeedController(); } public function testRssMethodLogsCorrectly(): void @@ -138,7 +144,7 @@ class FeedControllerTest extends TestCase $this->setupMockDatabase($testTicks); - $controller = new FeedController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new FeedController(); // Capture output to prevent headers/content from affecting test ob_start(); @@ -160,7 +166,7 @@ class FeedControllerTest extends TestCase $this->setupMockDatabase($testTicks); - $controller = new FeedController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new FeedController(); // Capture output to prevent headers/content from affecting test ob_start(); diff --git a/tests/Controller/HomeController/HomeControllerTest.php b/tests/Controller/HomeController/HomeControllerTest.php index 647e94f..6bfbf26 100644 --- a/tests/Controller/HomeController/HomeControllerTest.php +++ b/tests/Controller/HomeController/HomeControllerTest.php @@ -16,11 +16,6 @@ class HomeControllerTest extends TestCase mkdir($this->tempLogDir . '/logs', 0777, true); Log::init($this->tempLogDir . '/logs/tkr.log'); - // Set up global config for logging level (DEBUG = 1) - global $config; - $config = new stdClass(); - $config->logLevel = 1; // Allow DEBUG level logs - // Create mock PDO and PDOStatement $this->mockStatement = $this->createMock(PDOStatement::class); $this->mockPdo = $this->createMock(PDO::class); @@ -34,6 +29,17 @@ class HomeControllerTest extends TestCase $this->mockUser = new UserModel($this->mockPdo); $this->mockUser->displayName = 'Test User'; $this->mockUser->mood = '😊'; + + // Set up global $app for simplified dependency access + global $app; + $app = [ + 'db' => $this->mockPdo, + 'config' => $this->mockConfig, + 'user' => $this->mockUser, + ]; + + // Set log level on config for Log class + $this->mockConfig->logLevel = 1; // Allow DEBUG level logs } protected function tearDown(): void @@ -91,7 +97,7 @@ class HomeControllerTest extends TestCase { $this->setupMockDatabase([]); // Empty array = no ticks - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $data = $controller->getHomeData(1); // Should return proper structure @@ -118,7 +124,7 @@ class HomeControllerTest extends TestCase $this->setupMockDatabase($testTicks); - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $data = $controller->getHomeData(1); // Should return proper structure @@ -147,7 +153,7 @@ class HomeControllerTest extends TestCase ->method('execute') ->with([10, 10]); // itemsPerPage=10, page 2 = offset 10 - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $controller->getHomeData(2); // Page 2 } @@ -171,7 +177,7 @@ class HomeControllerTest extends TestCase && $params[1] === 'This is a test tick'; })); - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $postData = ['new_tick' => 'This is a test tick']; $result = $controller->processTick($postData); @@ -185,7 +191,7 @@ class HomeControllerTest extends TestCase // PDO shouldn't be called at all for empty content $this->mockPdo->expects($this->never())->method('prepare'); - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $postData = ['new_tick' => ' ']; // Just whitespace $result = $controller->processTick($postData); @@ -199,7 +205,7 @@ class HomeControllerTest extends TestCase // PDO shouldn't be called at all for missing field $this->mockPdo->expects($this->never())->method('prepare'); - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $postData = []; // No new_tick field $result = $controller->processTick($postData); @@ -219,7 +225,7 @@ class HomeControllerTest extends TestCase return $params[1] === 'This has whitespace'; // Should be trimmed })); - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $postData = ['new_tick' => ' This has whitespace ']; $result = $controller->processTick($postData); @@ -231,7 +237,7 @@ class HomeControllerTest extends TestCase { $this->setupMockDatabaseForInsert(false); // Will throw exception - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $postData = ['new_tick' => 'This will fail']; $result = $controller->processTick($postData); @@ -247,7 +253,7 @@ class HomeControllerTest extends TestCase ]; $this->setupMockDatabase($testTicks); - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $controller->getHomeData(1); // Check that logs were written @@ -263,7 +269,7 @@ class HomeControllerTest extends TestCase { $this->setupMockDatabaseForInsert(true); - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $postData = ['new_tick' => 'Test tick for logging']; $controller->processTick($postData); @@ -278,7 +284,7 @@ class HomeControllerTest extends TestCase public function testLoggingOnEmptyTick(): void { - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $postData = ['new_tick' => '']; $controller->processTick($postData); @@ -297,7 +303,7 @@ class HomeControllerTest extends TestCase { $this->setupMockDatabaseForInsert(false); - $controller = new HomeController($this->mockPdo, $this->mockConfig, $this->mockUser); + $controller = new HomeController(); $postData = ['new_tick' => 'This will fail']; $controller->processTick($postData); diff --git a/tests/Controller/LogController/LogControllerTest.php b/tests/Controller/LogController/LogControllerTest.php index 6c90c22..684e028 100644 --- a/tests/Controller/LogController/LogControllerTest.php +++ b/tests/Controller/LogController/LogControllerTest.php @@ -19,12 +19,20 @@ class LogControllerTest extends TestCase $this->originalGet = $_GET; $_GET = []; - // Mock global config - global $config; + // Set up global $app for simplified dependency access $mockPdo = $this->createMock(PDO::class); - $config = new ConfigModel($mockPdo); - $config->baseUrl = 'https://example.com'; - $config->basePath = '/tkr/'; + $mockConfig = new ConfigModel($mockPdo); + $mockConfig->baseUrl = 'https://example.com'; + $mockConfig->basePath = '/tkr/'; + + $mockUser = new UserModel($mockPdo); + + global $app; + $app = [ + 'db' => $mockPdo, + 'config' => $mockConfig, + 'user' => $mockUser, + ]; } protected function tearDown(): void @@ -51,10 +59,8 @@ class LogControllerTest extends TestCase public function testGetLogDataWithNoLogFiles(): void { - $mockPdo = $this->createMock(PDO::class); - $mockConfig = new ConfigModel($mockPdo); - $mockUser = new UserModel($mockPdo); - $controller = new LogController($mockPdo, $mockConfig, $mockUser, $this->tempLogDir); + // Uses global $app set up in setUp() + $controller = new LogController($this->tempLogDir); $data = $controller->getLogData(); // Should return empty log entries but valid structure @@ -85,10 +91,8 @@ class LogControllerTest extends TestCase file_put_contents($this->testLogFile, $logContent); - $mockPdo = $this->createMock(PDO::class); - $mockConfig = new ConfigModel($mockPdo); - $mockUser = new UserModel($mockPdo); - $controller = new LogController($mockPdo, $mockConfig, $mockUser, $this->tempLogDir); + // Uses global $app set up in setUp() + $controller = new LogController($this->tempLogDir); $data = $controller->getLogData(); // Should parse all valid entries and ignore invalid ones @@ -129,10 +133,8 @@ class LogControllerTest extends TestCase file_put_contents($this->testLogFile, $logContent); - $mockPdo = $this->createMock(PDO::class); - $mockConfig = new ConfigModel($mockPdo); - $mockUser = new UserModel($mockPdo); - $controller = new LogController($mockPdo, $mockConfig, $mockUser, $this->tempLogDir); + // Uses global $app set up in setUp() + $controller = new LogController($this->tempLogDir); $data = $controller->getLogData('ERROR'); // Should only include ERROR entries @@ -152,10 +154,8 @@ class LogControllerTest extends TestCase file_put_contents($this->testLogFile, $logContent); - $mockPdo = $this->createMock(PDO::class); - $mockConfig = new ConfigModel($mockPdo); - $mockUser = new UserModel($mockPdo); - $controller = new LogController($mockPdo, $mockConfig, $mockUser, $this->tempLogDir); + // Uses global $app set up in setUp() + $controller = new LogController($this->tempLogDir); $data = $controller->getLogData('', 'GET /admin'); // Should only include GET /admin entries @@ -175,10 +175,8 @@ class LogControllerTest extends TestCase file_put_contents($this->testLogFile, $logContent); - $mockPdo = $this->createMock(PDO::class); - $mockConfig = new ConfigModel($mockPdo); - $mockUser = new UserModel($mockPdo); - $controller = new LogController($mockPdo, $mockConfig, $mockUser, $this->tempLogDir); + // Uses global $app set up in setUp() + $controller = new LogController($this->tempLogDir); $data = $controller->getLogData('ERROR', 'GET /admin'); // Should only include entries matching both filters @@ -201,10 +199,8 @@ class LogControllerTest extends TestCase $rotatedLog2 = '[2025-01-31 12:00:00] WARNING: 127.0.0.1 - Rotated log entry 2'; file_put_contents($this->testLogFile . '.2', $rotatedLog2); - $mockPdo = $this->createMock(PDO::class); - $mockConfig = new ConfigModel($mockPdo); - $mockUser = new UserModel($mockPdo); - $controller = new LogController($mockPdo, $mockConfig, $mockUser, $this->tempLogDir); + // Uses global $app set up in setUp() + $controller = new LogController($this->tempLogDir); $data = $controller->getLogData(); // Should read from all log files, newest first @@ -226,10 +222,8 @@ class LogControllerTest extends TestCase file_put_contents($this->testLogFile, $logContent); - $mockPdo = $this->createMock(PDO::class); - $mockConfig = new ConfigModel($mockPdo); - $mockUser = new UserModel($mockPdo); - $controller = new LogController($mockPdo, $mockConfig, $mockUser, $this->tempLogDir); + // Uses global $app set up in setUp() + $controller = new LogController($this->tempLogDir); $data = $controller->getLogData(); // Should extract unique routes, sorted @@ -248,10 +242,8 @@ class LogControllerTest extends TestCase file_put_contents($this->testLogFile, $logContent); - $mockPdo = $this->createMock(PDO::class); - $mockConfig = new ConfigModel($mockPdo); - $mockUser = new UserModel($mockPdo); - $controller = new LogController($mockPdo, $mockConfig, $mockUser, $this->tempLogDir); + // Uses global $app set up in setUp() + $controller = new LogController($this->tempLogDir); $data = $controller->getLogData(); // Should only include valid entries, ignore invalid ones diff --git a/tests/Framework/Log/LogTest.php b/tests/Framework/Log/LogTest.php index 0f9e4d0..2b9fc56 100644 --- a/tests/Framework/Log/LogTest.php +++ b/tests/Framework/Log/LogTest.php @@ -43,10 +43,11 @@ class LogTest extends TestCase { Log::setRouteContext('GET /admin'); - // Create a mock config for log level - global $config; - $config = new stdClass(); - $config->logLevel = 1; // DEBUG level + // Create a mock app config for log level + global $app; + $app = [ + 'config' => (object)['logLevel' => 1] // DEBUG level + ]; Log::debug('Test message'); @@ -61,9 +62,10 @@ class LogTest extends TestCase { Log::setRouteContext(''); - global $config; - $config = new stdClass(); - $config->logLevel = 1; + global $app; + $app = [ + 'config' => (object)['logLevel' => 1] + ]; Log::info('Test without route'); @@ -78,9 +80,10 @@ class LogTest extends TestCase public function testLogLevelFiltering(): void { - global $config; - $config = new stdClass(); - $config->logLevel = 3; // WARNING level + global $app; + $app = [ + 'config' => (object)['logLevel' => 3] // WARNING level + ]; Log::debug('Debug message'); // Should be filtered out Log::info('Info message'); // Should be filtered out @@ -99,9 +102,10 @@ class LogTest extends TestCase { Log::setRouteContext('POST /admin'); - global $config; - $config = new stdClass(); - $config->logLevel = 1; + global $app; + $app = [ + 'config' => (object)['logLevel' => 1] + ]; Log::error('Test error message'); @@ -129,9 +133,10 @@ class LogTest extends TestCase public function testLogRotation(): void { - global $config; - $config = new stdClass(); - $config->logLevel = 1; + global $app; + $app = [ + 'config' => (object)['logLevel' => 1] + ]; // Create a log file with exactly 1000 lines (the rotation threshold) $logLines = str_repeat("[2025-01-31 12:00:00] INFO: 127.0.0.1 - Test line\n", 1000); @@ -154,9 +159,11 @@ class LogTest extends TestCase public function testDefaultLogLevelWhenConfigMissing(): void { - // Clear global config - global $config; - $config = null; + // Set up config without logLevel property (simulates missing config value) + global $app; + $app = [ + 'config' => (object)[] // Empty config object, no logLevel property + ]; // Should not throw errors and should default to INFO level Log::debug('Debug message'); // Should be filtered out (default INFO level = 2) diff --git a/tests/Framework/Util/UtilTest.php b/tests/Framework/Util/UtilTest.php index e4bdfb5..5e60d77 100644 --- a/tests/Framework/Util/UtilTest.php +++ b/tests/Framework/Util/UtilTest.php @@ -73,4 +73,134 @@ final class UtilTest extends TestCase $this->assertEquals($expected, $result); } + // Test data for escape_html function + public static function escapeHtmlProvider(): array { + return [ + 'basic HTML' => ['', '<script>alert("xss")</script>'], + 'quotes' => ['He said "Hello" & she said \'Hi\'', 'He said "Hello" & she said 'Hi''], + 'empty string' => ['', ''], + 'normal text' => ['Hello World', 'Hello World'], + 'ampersand' => ['Tom & Jerry', 'Tom & Jerry'], + 'unicode' => ['🚀 emoji & text', '🚀 emoji & text'], + ]; + } + + #[DataProvider('escapeHtmlProvider')] + public function testEscapeHtml(string $input, string $expected): void { + $result = Util::escape_html($input); + $this->assertEquals($expected, $result); + } + + // Test data for escape_xml function + public static function escapeXmlProvider(): array { + return [ + 'basic XML' => ['content', '<tag attr="value">content</tag>'], + 'quotes and ampersand' => ['Title & "Subtitle"', 'Title & "Subtitle"'], + 'empty string' => ['', ''], + 'normal text' => ['Hello World', 'Hello World'], + 'unicode' => ['🎵 music & notes', '🎵 music & notes'], + ]; + } + + #[DataProvider('escapeXmlProvider')] + public function testEscapeXml(string $input, string $expected): void { + $result = Util::escape_xml($input); + $this->assertEquals($expected, $result); + } + + // Test data for linkify function + public static function linkifyProvider(): array { + return [ + 'simple URL' => [ + 'Check out https://example.com for more info', + 'Check out https://example.com for more info', + false // not strict accessibility + ], + 'URL with path' => [ + 'Visit https://example.com/path/to/page', + 'Visit https://example.com/path/to/page', + false + ], + 'multiple URLs' => [ + 'See https://example.com and https://other.com', + 'See https://example.com and https://other.com', + false + ], + 'URL with punctuation' => [ + 'Check https://example.com.', + 'Check https://example.com', + false + ], + 'no URL' => [ + 'Just some regular text', + 'Just some regular text', + false + ], + 'strict accessibility mode' => [ + 'Visit https://example.com now', + 'Visit https://example.com now', + true // strict accessibility + ], + ]; + } + + #[DataProvider('linkifyProvider')] + public function testLinkify(string $input, string $expected, bool $strictAccessibility): void { + // Set up global $app with config + global $app; + $app = [ + 'config' => (object)['strictAccessibility' => $strictAccessibility] + ]; + + $result = Util::linkify($input); + $this->assertEquals($expected, $result); + } + + public function testLinkifyNoNewWindow(): void { + // Test linkify without new window + global $app; + $app = [ + 'config' => (object)['strictAccessibility' => false] + ]; + + $input = 'Visit https://example.com'; + $expected = 'Visit https://example.com'; + + $result = Util::linkify($input, false); // no new window + $this->assertEquals($expected, $result); + } + + public function testGetClientIp(): void { + // Test basic case with REMOTE_ADDR + $_SERVER['REMOTE_ADDR'] = '192.168.1.100'; + unset($_SERVER['HTTP_CLIENT_IP'], $_SERVER['HTTP_X_FORWARDED_FOR'], $_SERVER['HTTP_X_REAL_IP']); + + $result = Util::getClientIp(); + $this->assertEquals('192.168.1.100', $result); + } + + public function testGetClientIpWithForwardedHeaders(): void { + // Test precedence: HTTP_CLIENT_IP > HTTP_X_FORWARDED_FOR > HTTP_X_REAL_IP > REMOTE_ADDR + $_SERVER['HTTP_CLIENT_IP'] = '10.0.0.1'; + $_SERVER['HTTP_X_FORWARDED_FOR'] = '10.0.0.2'; + $_SERVER['HTTP_X_REAL_IP'] = '10.0.0.3'; + $_SERVER['REMOTE_ADDR'] = '10.0.0.4'; + + $result = Util::getClientIp(); + $this->assertEquals('10.0.0.1', $result); // Should use HTTP_CLIENT_IP + } + + public function testGetClientIpUnknown(): void { + // Test when no IP is available + unset($_SERVER['HTTP_CLIENT_IP'], $_SERVER['HTTP_X_FORWARDED_FOR'], $_SERVER['HTTP_X_REAL_IP'], $_SERVER['REMOTE_ADDR']); + + $result = Util::getClientIp(); + $this->assertEquals('unknown', $result); + } + + protected function tearDown(): void { + // Clean up $_SERVER after IP tests + unset($_SERVER['HTTP_CLIENT_IP'], $_SERVER['HTTP_X_FORWARDED_FOR'], $_SERVER['HTTP_X_REAL_IP'], $_SERVER['REMOTE_ADDR']); + } + } \ No newline at end of file