Fixes for installer/modules race condition (#883)

app/Contracts/Migration.php

@@ -2,8 +2,10 @@
 
 namespace App\Contracts;
 
+use App\Models\Module;
 use App\Support\Database;
-use DB;
+use Exception;
+use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Validator;
 use Illuminate\Validation\ValidationException;
@@ -27,6 +29,29 @@ abstract class Migration extends \Illuminate\Database\Migrations\Migration
     {
     }
 
+    /**
+     * Add a module and enable it
+     *
+     * @param array $attrs
+     */
+    public function addModule(array $attrs)
+    {
+        $module = array_merge([
+            'enabled'    => true,
+            'created_at' => DB::raw('NOW()'),
+            'updated_at' => DB::raw('NOW()'),
+        ], $attrs);
+
+        try {
+            DB::table('modules')->insert($module);
+        } catch (Exception $e) {
+            // setting already exists, just ignore it
+            if ($e->getCode() === 23000) {
+                return;
+            }
+        }
+    }
+
     /**
      * Seed a YAML file into the database
      *
@@ -37,7 +62,7 @@ abstract class Migration extends \Illuminate\Database\Migrations\Migration
         try {
             $path = base_path($file);
             Database::seed_from_yaml_file($path, false);
-        } catch (\Exception $e) {
+        } catch (Exception $e) {
             Log::error('Unable to load '.$file.' file');
             Log::error($e);
         }
@@ -54,7 +79,7 @@ abstract class Migration extends \Illuminate\Database\Migrations\Migration
         foreach ($rows as $row) {
             try {
                 DB::table($table)->insert($row);
-            } catch (\Exception $e) {
+            } catch (Exception $e) {
                 // setting already exists, just ignore it
                 if ($e->getCode() === 23000) {
                     continue;

app/Database/migrations/2020_09_30_081536_create_modules_table.php

@@ -1,6 +1,6 @@
 <?php
 
-use Illuminate\Database\Migrations\Migration;
+use App\Contracts\Migration;
 use Illuminate\Database\Schema\Blueprint;
 use Illuminate\Support\Facades\Schema;
 
@@ -19,6 +19,12 @@ class CreateModulesTable extends Migration
             $table->boolean('enabled')->default(1);
             $table->timestamps();
         });
+
+        $this->addModule(['name' => 'Awards']);
+        $this->addModule(['name' => 'Sample']);
+        $this->addModule(['name' => 'VMSAcars']);
+        $this->addModule(['name' => 'Vacentral']);
+        $this->addModule(['name' => 'TestModule']);
     }
 
     /**

app/Database/seeds/DatabaseSeeder.php

@@ -1,15 +1,21 @@
 <?php
 
+use App\Services\Installer\MigrationService;
 use App\Services\Installer\SeederService;
 use Illuminate\Database\Seeder;
 
 class DatabaseSeeder extends Seeder
 {
-    private $seederService;
+    /** @var MigrationService */
+    private $migrationSvc;
+
+    /** @var SeederService */
+    private $seederSvc;
 
     public function __construct()
     {
-        $this->seederService = app(SeederService::class);
+        $this->migrationSvc = app(MigrationService::class);
+        $this->seederSvc = app(SeederService::class);
     }
 
     /**
@@ -19,6 +25,12 @@ class DatabaseSeeder extends Seeder
      */
     public function run()
     {
-        $this->seederService->syncAllSeeds();
+        // Make sure any migrations that need to be run are run/cleared out
+        if ($this->migrationSvc->migrationsAvailable()) {
+            $this->migrationSvc->runAllMigrations();
+        }
+
+        // Then sync all of the seeds
+        $this->seederSvc->syncAllSeeds();
     }
 }

app/Models/Module.php

@@ -3,9 +3,13 @@
 namespace App\Models;
 
 use App\Contracts\Model;
+use Carbon\Carbon;
 
 /**
- * Class ModuleManager
+ * @property string name
+ * @property bool   enabled
+ * @property Carbon created_at
+ * @property Carbon updated_at
  */
 class Module extends Model
 {

app/Services/Installer/DatabaseService.php

@@ -47,6 +47,7 @@ class DatabaseService extends Service
         } catch (\PDOException $e) {
             throw $e;
         }
+
         return true;
     }
 

app/Services/Installer/MigrationService.php

@@ -3,13 +3,16 @@
 namespace App\Services\Installer;
 
 use App\Contracts\Service;
+use Exception;
+use Illuminate\Database\Migrations\Migrator;
 use Illuminate\Support\Facades\App;
 use Illuminate\Support\Facades\Artisan;
+use Illuminate\Support\Facades\Log;
 use Nwidart\Modules\Facades\Module;
 
 class MigrationService extends Service
 {
-    protected function getMigrator()
+    protected function getMigrator(): Migrator
     {
         $m = app('migrator');
         $m->setConnection(config('database.default'));
@@ -36,8 +39,6 @@ class MigrationService extends Service
             }
         }
 
-        // Log::info('Update - migration paths', $paths);
-
         return $paths;
     }
 
@@ -49,10 +50,25 @@ class MigrationService extends Service
         $migrator = $this->getMigrator();
         $migration_dirs = $this->getMigrationPaths();
 
-        $files = $migrator->getMigrationFiles(array_values($migration_dirs));
+        $availMigrations = [];
-        $availMigrations = array_diff(array_keys($files), $migrator->getRepository()->getRan());
+        $runFiles = [];
 
-        // Log::info('Migrations available:', $availMigrations);
+        try {
+            $runFiles = $migrator->getRepository()->getRan();
+        } catch (Exception $e) {
+        } // Skip database run initialized
+
+        $files = $migrator->getMigrationFiles(array_values($migration_dirs));
+
+        foreach ($files as $filename => $filepath) {
+            if (in_array($filename, $runFiles, true)) {
+                continue;
+            }
+
+            $availMigrations[] = $filepath;
+        }
+
+        Log::info('Migrations available:', $availMigrations);
 
         return $availMigrations;
     }
@@ -63,11 +79,23 @@ class MigrationService extends Service
      */
     public function runAllMigrations()
     {
+        // A little ugly, run the main migration first, this makes sure the migration table is there
         $output = '';
 
         Artisan::call('migrate');
         $output .= trim(Artisan::output());
 
-        return $output;
+        // Then get any remaining migrations that are left over
+        // Due to caching or whatever reason, the migrations are not all loaded when Artisan first
+        // runs. This is likely a side effect of the database being used as the module activator,
+        // and the list of migrations being pulled before the initial modules are populated
+        $migrator = $this->getMigrator();
+        $availMigrations = $this->migrationsAvailable();
+
+        Log::info('Running '.count($availMigrations).' available migrations');
+        $ret = $migrator->run($availMigrations);
+        Log::info('Ran '.count($ret).' migrations');
+
+        return $output."\n".implode("\n", $ret);
     }
 }

app/Services/Installer/SeederService.php

@@ -56,10 +56,10 @@ class SeederService extends Service
      */
     public function syncAllSeeds(): void
     {
-        $this->syncAllYamlFileSeeds();
         $this->syncAllSettings();
         $this->syncAllPermissions();
         $this->syncAllModules();
+        $this->syncAllYamlFileSeeds();
     }
 
     /**

app/Services/ModuleService.php

@@ -125,8 +125,12 @@ class ModuleService extends Service
                 'name'    => $module_name,
                 'enabled' => 1,
             ]);
+
+            Artisan::call('module:migrate '.$module_name);
+
             return true;
         }
+
         return false;
     }
 
@@ -240,6 +244,11 @@ class ModuleService extends Service
         $module->update([
             'enabled' => $status,
         ]);
+
+        if ($status === true) {
+            Artisan::call('module:migrate '.$module->name);
+        }
+
         return true;
     }
 

app/Support/Modules/DatabaseActivator.php

@@ -53,6 +53,20 @@ class DatabaseActivator implements ActivatorInterface
         $this->path = $path;
     }
 
+    /**
+     * @param string $name
+     *
+     * @return \App\Models\Module|null
+     */
+    public function getModuleByName(string $name): ?\App\Models\Module
+    {
+        try {
+            return \App\Models\Module::where(['name' => $name])->first();
+        } catch (Exception $e) { // Catch any database/connection errors
+            return null;
+        }
+    }
+
     /**
      * Get modules statuses, from the database
      *
@@ -66,6 +80,7 @@ class DatabaseActivator implements ActivatorInterface
             foreach ($modules as $i) {
                 $retVal[$i->name] = $i->enabled;
             }
+
             return $retVal;
         } catch (Exception $e) {
             return [];
@@ -85,7 +100,7 @@ class DatabaseActivator implements ActivatorInterface
      */
     public function enable(Module $module): void
     {
-        $this->setActiveByName($module->getName(), true);
+        $this->setActive($module, true);
     }
 
     /**
@@ -93,7 +108,7 @@ class DatabaseActivator implements ActivatorInterface
      */
     public function disable(Module $module): void
     {
-        $this->setActiveByName($module->getName(), false);
+        $this->setActive($module, false);
     }
 
     /**
@@ -102,15 +117,12 @@ class DatabaseActivator implements ActivatorInterface
      */
     public function hasStatus(Module $module, bool $status): bool
     {
-        try {
+        $module = $this->getModuleByName($module->getName());
-            $module = (new \App\Models\Module())->where('name', $module->getName());
+        if (!$module) {
-            if ($module->exists()) {
-                return $module->first()->enabled == 1;
-            }
-            return false;
-        } catch (Exception $e) {
             return false;
         }
+
+        return $module->enabled;
     }
 
     /**
@@ -118,7 +130,13 @@ class DatabaseActivator implements ActivatorInterface
      */
     public function setActive(Module $module, bool $active): void
     {
-        $this->setActiveByName($module->getName(), $active);
+        $module = $this->getModuleByName($module->getName());
+        if (!$module) {
+            return;
+        }
+
+        $module->enabled = $active;
+        $module->save();
     }
 
     /**
@@ -126,12 +144,13 @@ class DatabaseActivator implements ActivatorInterface
      */
     public function setActiveByName(string $name, bool $status): void
     {
-        $module = (new \App\Models\Module())->where('name', $name);
+        $module = $this->getModuleByName($name);
-        if ($module->exists()) {
+        if (!$module) {
-            $module->update([
+            return;
-                'status' => $status,
-            ]);
         }
+
+        $module->enabled = $status;
+        $module->save();
     }
 
     /**