refactor-app-initialization (#51)

Separate database migrations from other database initialization functions.
Move some initialization directly into index to keep classes targeted.
Simplify setup validation and redirection logic.
Clean up comments.

Reviewed-on: https://gitea.subcultureofone.org/greg/tkr/pulls/51
Co-authored-by: Greg Sarjeant <greg@subcultureofone.org>
Co-committed-by: Greg Sarjeant <greg@subcultureofone.org>

public/index.php

@@ -1,4 +1,8 @@
 <?php
+/*
+ *  Initialize fundamental configuration
+ */
+
 // Store and validate request data
 $method = $_SERVER['REQUEST_METHOD'];
 $request = $_SERVER['REQUEST_URI'];
@@ -14,7 +18,11 @@ if (preg_match('/\.php$/', $path)) {
 // Define base paths and load classes
 include_once(dirname(dirname(__FILE__)) . "/config/bootstrap.php");
 
-// Check prerequisites.
+/*
+ *  Validate application state before processing request
+ */
+
+// Check prerequisites
 $prerequisites = new Prerequisites();
 $results = $prerequisites->validate();
 if (count($prerequisites->getErrors()) > 0) {
@@ -22,28 +30,46 @@ if (count($prerequisites->getErrors()) > 0) {
     exit;
 }
 
-// Do any necessary database migrations
+// Connect to the database
-$dbMgr = new Database();
+try {
-$dbMgr->migrate();
+    // SQLite will just create this if it doesn't exist.
-
+    $db = new PDO("sqlite:" . DB_FILE);
-// Make sure the initial setup is complete
+    $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
-// unless we're already heading to setup
+    $db->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC);
-//
+} catch (PDOException $e) {
-// TODO: Consider simplifying this.
+    throw new SetupException(
-// Might not need the custom exception now that the prereq checker is more robust.
+        "Database connection failed: " . $e->getMessage(),
-if (!(preg_match('/setup$/', $path))) {
+        'database_connection',
-    try {
+        0,
-        // Make sure setup has been completed
+        $e
-        $dbMgr->confirmSetup();
+    );
-    } catch (SetupException $e) {
-        $e->handle();
-        exit;
-    }
 }
 
+// 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
+if (!(preg_match('/setup$/', $path))) {
+    // Make sure required tables (user, settings) are populated
+    $user_count = (int) $db->query("SELECT COUNT(*) FROM user")->fetchColumn();
+    $settings_count = (int) $db->query("SELECT COUNT(*) FROM settings")->fetchColumn();
+
+    // If either required table has no records, redirect to setup.
+    if ($user_count === 0 || $settings_count === 0){
+        $init = require APP_ROOT . '/config/init.php';
+        header('Location: ' . $init['base_path'] . 'setup');
+        exit;
+    };
+}
+
+/*
+ *  Begin processing request
+ */
+
 // Initialize application context with all dependencies
 global $app;
-$db = Database::get();
+
 $app = [
     'db' => $db,
     'config' => (new ConfigModel($db))->loadFromDatabase(),
@@ -51,7 +77,6 @@ $app = [
 ];
 
 // Start a session and generate a CSRF Token
-// if there isn't already an active session
 Session::start();
 Session::generateCsrfToken();
 
@@ -75,7 +100,7 @@ if ($method === 'POST' && $path != 'setup') {
         if (!Session::isValid($_POST['csrf_token'])) {
             // Invalid session - redirect to /login
             Log::info('Attempt to POST with invalid session. Redirecting to login.');
-            header('Location: ' . Util::buildRelativeUrl($config->basePath, 'login'));
+            header('Location: ' . Util::buildRelativeUrl($app->config->basePath, 'login'));
             exit;
         }
     } else {

src/Framework/Exception/SetupException.php

@@ -20,6 +20,8 @@ class SetupException extends Exception {
             // 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':
@@ -29,19 +31,6 @@ class SetupException extends Exception {
                 echo "<h1>Configuration Error</h1>";
                 echo "<p>" . Util::escape_html($this->setupIssue) . '-' . Util::escape_html($this->getMessage()) . "</p>";
                 exit;
-            case 'table_contents':
-                // Recoverable error.
-                // Redirect to setup if we aren't already headed there.
-                // NOTE: Just read directly from init.php instead of
-                //       trying to use the config object. This is the initial
-                //       setup. It shouldn't assume any data can be loaded.
-                $init = require APP_ROOT . '/config/init.php';
-                $currentPath = trim(parse_url($_SERVER['REQUEST_URI'], PHP_URL_PATH), '/');
-
-                if (strpos($currentPath, 'setup') === false) {
-                    header('Location: ' . $init['base_path'] . 'setup');
-                    exit;
-                }
         }
     }
 

src/Framework/Migrator/Migrator.php

@@ -1,35 +1,11 @@
 <?php
-class Database{
+class Migrator{
-    // TODO = Make this not static
+    public function __construct(private PDO $db) {}
-    public static function get(): PDO {
-        try {
-            // 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
-            );
-        }
 
-        return $db;
+    // The database version is an int stored as PRAGMA user_version.
-    }
+    // It corresponds to the most recent migration file applied to the db.
-
-    public function validate(): void{
-        $this->validateTableContents();
-    }
-
-    // The database version will just be an int
-    // stored as PRAGMA user_version. It will
-    // correspond to the most recent migration file applied to the db.
     private function getVersion(): int {
-        $db = self::get();
+        return $this->db->query("PRAGMA user_version")->fetchColumn() ?? 0;
-
-        return $db->query("PRAGMA user_version")->fetchColumn() ?? 0;
     }
 
     private function migrationNumberFromFile(string $filename): int {
@@ -48,8 +24,7 @@ class Database{
             );
         }
 
-        $db = self::get();
+        $this->db->exec("PRAGMA user_version = $newVersion");
-        $db->exec("PRAGMA user_version = $newVersion");
     }
 
     private function getPendingMigrations(): array {
@@ -79,8 +54,7 @@ class Database{
         Log::info("Found " . count($migrations) . " pending migrations.");
         Log::info("Updating database. Current Version: " . $this->getVersion());
 
-        $db = self::get();
+        $this->db->beginTransaction();
-        $db->beginTransaction();
 
         try {
             foreach ($migrations as $version => $file) {
@@ -100,7 +74,7 @@ class Database{
                 foreach ($statements as $statement){
                     if (!empty($statement)){
                         Log::debug("Migration statement: {$statement}");
-                        $db->exec($statement);
+                        $this->db->exec($statement);
                     }
                 }
 
@@ -108,13 +82,13 @@ class Database{
             }
 
             // Update db version
-            $db->commit();
+            $this->db->commit();
             $this->setVersion($version);
 
             Log::info("Applied " . count($migrations) . " migrations.");
             Log::info("Updated database version to " . $this->getVersion());
         } catch (Exception $e) {
-            $db->rollBack();
+            $this->db->rollBack();
             throw new SetupException(
                 "Migration failed: $filename",
                 'db_migration',
@@ -123,22 +97,4 @@ class Database{
             );
         }
     }
-
-    // make sure tables that need to be seeded have been
-    public function confirmSetup(): void {
-        $db = self::get();
-
-        // make sure required tables (user, settings) are populated
-        $user_count = (int) $db->query("SELECT COUNT(*) FROM user")->fetchColumn();
-        $settings_count = (int) $db->query("SELECT COUNT(*) FROM settings")->fetchColumn();
-
-        // If either required table has no records, throw an exception.
-        // This will be caught and redirect to setup.
-        if ($user_count === 0 || $settings_count === 0){
-            throw new SetupException(
-                "Required tables aren't populated. Please complete setup",
-                'table_contents',
-            );
-        };
-    }
 }