Optimize initialization logic and simplify error handling. (#54)
Consolidate prerequisite tests - include database migrations. Remove SetupException and fold database initialization error handling into prereqs. Reviewed-on: https://gitea.subcultureofone.org/greg/tkr/pulls/54 Co-authored-by: Greg Sarjeant <greg@subcultureofone.org> Co-committed-by: Greg Sarjeant <greg@subcultureofone.org>
This commit is contained in:
		
							parent
							
								
									7816581216
								
							
						
					
					
						commit
						51abf33ad1
					
				| @ -22,32 +22,15 @@ include_once(dirname(dirname(__FILE__)) . "/config/bootstrap.php"); | |||||||
|  *  Validate application state before processing request |  *  Validate application state before processing request | ||||||
|  */ |  */ | ||||||
| 
 | 
 | ||||||
| // Check prerequisites
 | // Check prerequisites (includes database connection and migrations)
 | ||||||
| $prerequisites = new Prerequisites(); | $prerequisites = new Prerequisites(); | ||||||
| $results = $prerequisites->validate(); | if (!$prerequisites->validate()) { | ||||||
| if (count($prerequisites->getErrors()) > 0) { |     $prerequisites->generateWebSummary(); | ||||||
|     $prerequisites->generateWebSummary($results); |  | ||||||
|     exit; |     exit; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| // Connect to the database
 | // Get the working database connection from prerequisites
 | ||||||
| try { | $db = $prerequisites->getDatabase(); | ||||||
|     // SQLite will just create this if it doesn't exist.
 |  | ||||||
|     $db = new PDO("sqlite:" . DB_FILE); |  | ||||||
|     $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); |  | ||||||
|     $db->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC); |  | ||||||
| } catch (PDOException $e) { |  | ||||||
|     throw new SetupException( |  | ||||||
|         "Database connection failed: " . $e->getMessage(), |  | ||||||
|         'database_connection', |  | ||||||
|         0, |  | ||||||
|         $e |  | ||||||
|     ); |  | ||||||
| } |  | ||||||
| 
 |  | ||||||
| // Do any necessary database migrations
 |  | ||||||
| $migrator = new Migrator($db); |  | ||||||
| $migrator->migrate(); |  | ||||||
| 
 | 
 | ||||||
| // Make sure the initial setup is complete unless we're already heading to setup
 | // Make sure the initial setup is complete unless we're already heading to setup
 | ||||||
| if (!(preg_match('/setup$/', $path))) { | if (!(preg_match('/setup$/', $path))) { | ||||||
|  | |||||||
| @ -1,38 +0,0 @@ | |||||||
| <?php |  | ||||||
| // Define an exception for validation errors
 |  | ||||||
| class SetupException extends Exception { |  | ||||||
|     private $setupIssue; |  | ||||||
| 
 |  | ||||||
|     public function __construct(string $message, string $setupIssue = '', int $code = 0, Throwable $previous = null) { |  | ||||||
|         parent::__construct($message, $code, $previous); |  | ||||||
|         $this->setupIssue = $setupIssue; |  | ||||||
|     } |  | ||||||
| 
 |  | ||||||
|     // Exception handler
 |  | ||||||
|     // Exceptions don't generally define their own handlers,
 |  | ||||||
|     // but this is a very specific case.
 |  | ||||||
|     public function handle(){ |  | ||||||
|         // try to log the error, but keep going if it fails
 |  | ||||||
|         try { |  | ||||||
|             Log::error($this->setupIssue . ", " . $this->getMessage()); |  | ||||||
|         } catch (Exception $e) { |  | ||||||
|             // Do nothing and move on to the normal error handling
 |  | ||||||
|             // We don't want to short-circuit this if there's a problem logging
 |  | ||||||
|         } |  | ||||||
| 
 |  | ||||||
|         // TODO: This doesn't need to be a switch anymore
 |  | ||||||
|         //       May not need to exist at all
 |  | ||||||
|         switch ($this->setupIssue){ |  | ||||||
|             case 'database_connection': |  | ||||||
|             case 'db_migration': |  | ||||||
|                 // Unrecoverable errors.
 |  | ||||||
|                 // Show error message and exit
 |  | ||||||
|                 http_response_code(500); |  | ||||||
|                 echo "<h1>Configuration Error</h1>"; |  | ||||||
|                 echo "<p>" . Util::escape_html($this->setupIssue) . '-' . Util::escape_html($this->getMessage()) . "</p>"; |  | ||||||
|                 exit; |  | ||||||
|         } |  | ||||||
|     } |  | ||||||
| 
 |  | ||||||
| 
 |  | ||||||
| } |  | ||||||
| @ -18,9 +18,8 @@ class Migrator{ | |||||||
|         $currentVersion = $this->getVersion(); |         $currentVersion = $this->getVersion(); | ||||||
| 
 | 
 | ||||||
|         if ($newVersion <= $currentVersion){ |         if ($newVersion <= $currentVersion){ | ||||||
|             throw new SetupException( |             throw new Exception( | ||||||
|                 "New version ($newVersion) must be greater than current version ($currentVersion)", |                 "New version ($newVersion) must be greater than current version ($currentVersion)" | ||||||
|                 'db_migration' |  | ||||||
|             ); |             ); | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
| @ -89,9 +88,8 @@ class Migrator{ | |||||||
|             Log::info("Updated database version to " . $this->getVersion()); |             Log::info("Updated database version to " . $this->getVersion()); | ||||||
|         } catch (Exception $e) { |         } catch (Exception $e) { | ||||||
|             $this->db->rollBack(); |             $this->db->rollBack(); | ||||||
|             throw new SetupException( |             throw new Exception( | ||||||
|                 "Migration failed: $filename", |                 "Migration failed: $filename - " . $e->getMessage(), | ||||||
|                 'db_migration', |  | ||||||
|                 0, |                 0, | ||||||
|                 $e |                 $e | ||||||
|             ); |             ); | ||||||
|  | |||||||
| @ -16,6 +16,7 @@ class Prerequisites { | |||||||
|     private $logFile; |     private $logFile; | ||||||
|     private $isCli; |     private $isCli; | ||||||
|     private $isWeb; |     private $isWeb; | ||||||
|  |     private $database = null; | ||||||
| 
 | 
 | ||||||
|     public function __construct() { |     public function __construct() { | ||||||
|         $this->isCli = php_sapi_name() === 'cli'; |         $this->isCli = php_sapi_name() === 'cli'; | ||||||
| @ -377,12 +378,68 @@ class Prerequisites { | |||||||
|             ); |             ); | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         return $canCreateDb; |         if (!$canCreateDb) { | ||||||
|  |             return false; | ||||||
|  |         } | ||||||
|  | 
 | ||||||
|  |         // Test database connection
 | ||||||
|  |         try { | ||||||
|  |             $db = new PDO("sqlite:" . $dbFile); | ||||||
|  |             $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); | ||||||
|  |             $db->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC); | ||||||
|  |              | ||||||
|  |             // Test basic query to ensure database is functional
 | ||||||
|  |             $db->query("SELECT 1")->fetchColumn(); | ||||||
|  |              | ||||||
|  |             $this->addCheck( | ||||||
|  |                 'Database Connection', | ||||||
|  |                 true, | ||||||
|  |                 'Successfully connected to database' | ||||||
|  |             ); | ||||||
|  |              | ||||||
|  |             // Store working database connection
 | ||||||
|  |             $this->database = $db; | ||||||
|  |              | ||||||
|  |             // Test migrations
 | ||||||
|  |             return $this->checkMigrations($db); | ||||||
|  |              | ||||||
|  |         } catch (PDOException $e) { | ||||||
|  |             $this->addCheck( | ||||||
|  |                 'Database Connection', | ||||||
|  |                 false, | ||||||
|  |                 'Failed to connect: ' . $e->getMessage(), | ||||||
|  |                 'error' | ||||||
|  |             ); | ||||||
|  |             return false; | ||||||
|  |         } | ||||||
|  |     } | ||||||
|  |      | ||||||
|  |     private function checkMigrations($db) { | ||||||
|  |         try { | ||||||
|  |             $migrator = new Migrator($db); | ||||||
|  |             $migrator->migrate(); | ||||||
|  |              | ||||||
|  |             $this->addCheck( | ||||||
|  |                 'Database Migrations', | ||||||
|  |                 true, | ||||||
|  |                 'All database migrations applied successfully' | ||||||
|  |             ); | ||||||
|  |             return true; | ||||||
|  |              | ||||||
|  |         } catch (Exception $e) { | ||||||
|  |             $this->addCheck( | ||||||
|  |                 'Database Migrations', | ||||||
|  |                 false, | ||||||
|  |                 'Migration failed: ' . $e->getMessage(), | ||||||
|  |                 'error' | ||||||
|  |             ); | ||||||
|  |             return false; | ||||||
|  |         } | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     // validate prereqs
 |     // validate prereqs
 | ||||||
|     // runs on each request and can be run from CLI
 |     // runs on each request and can be run from CLI
 | ||||||
|     public function validate() { |     public function validate(): bool { | ||||||
|         $this->log("=== tkr prerequisites check started at " . date('Y-m-d H:i:s') . " ===", true); |         $this->log("=== tkr prerequisites check started at " . date('Y-m-d H:i:s') . " ===", true); | ||||||
| 
 | 
 | ||||||
|         if ($this->isCli) { |         if ($this->isCli) { | ||||||
| @ -406,7 +463,8 @@ class Prerequisites { | |||||||
|             $this->generateCliSummary($results); |             $this->generateCliSummary($results); | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         return $results; |         // Return true only if no errors occurred
 | ||||||
|  |         return count($this->errors) === 0; | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     /** |     /** | ||||||
| @ -567,4 +625,14 @@ class Prerequisites { | |||||||
|     public function getWarnings() { |     public function getWarnings() { | ||||||
|         return $this->warnings; |         return $this->warnings; | ||||||
|     } |     } | ||||||
|  |      | ||||||
|  |     /** | ||||||
|  |      * Get working database connection (only call after validate() returns true) | ||||||
|  |      */ | ||||||
|  |     public function getDatabase(): PDO { | ||||||
|  |         if ($this->database === null) { | ||||||
|  |             throw new RuntimeException('Database not available - call validate() first'); | ||||||
|  |         } | ||||||
|  |         return $this->database; | ||||||
|  |     } | ||||||
| } | } | ||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user