diff --git a/CHANGELOG.md b/CHANGELOG.md index 316a5ba..658db4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ All notable changes to `laravel-ddd` will be documented in this file. +## [Unversioned] +### Changed +- Implement more robust handling of base models when generating a domain model with `ddd:model`: + - If the configured `ddd.base_model` exists (evaluated using `class_exists`), base model generation is skipped. + - If `ddd.base_model` does not exist and falls under a domain namespace, base model will be generated. + - Falling under a domain namespace means `Domain\**\Models\SomeBaseModel`. + - For example, if `ddd.base_model` were set to `App\Models\CustomAppBaseModel` or `Illuminate\Database\Eloquent\NonExistentModel`, they fall outside of the domain namespace and won't be generated on your behalf. + +### Fixed +- Resolve long-standing issue where `ddd:model` would not properly detect whether the configured `ddd.base_model` already exists, leading to unpredictable results when `ddd.base_model` deviated from the default `Domain\Shared\Models\BaseModel`. + +### Chore +- Update composer dependencies. + ## [0.7.0] - 2023-10-22 ### Added - Formal support for subdomains (nested domains). For example, to generate model `Domain\Reporting\Internal\Models\InvoiceReport`, the domain argument can be specified with dot notation: `ddd:model Reporting.Internal InvoiceReport`. Specifying `Reporting/Internal` or `Reporting\\Internal` will also be accepted and normalized to dot notation internally. diff --git a/composer.json b/composer.json index 76d4256..49a48f4 100644 --- a/composer.json +++ b/composer.json @@ -45,6 +45,7 @@ "Lunarstorm\\LaravelDDD\\Tests\\": "tests", "App\\": "vendor/orchestra/testbench-core/laravel/app/", "Database\\Factories\\": "vendor/orchestra/testbench-core/laravel/database/factories/", + "Database\\Seeders\\": "vendor/orchestra/testbench-core/laravel/database/seeders/", "Domain\\": "vendor/orchestra/testbench-core/laravel/src/Domain/" } }, diff --git a/src/Commands/MakeModel.php b/src/Commands/MakeModel.php index 51716a4..3239bfe 100644 --- a/src/Commands/MakeModel.php +++ b/src/Commands/MakeModel.php @@ -2,6 +2,7 @@ namespace Lunarstorm\LaravelDDD\Commands; +use Lunarstorm\LaravelDDD\Support\DomainResolver; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputOption; @@ -48,28 +49,65 @@ protected function getRelativeDomainNamespace(): string return config('ddd.namespaces.models', 'Models'); } + protected function preparePlaceholders(): array + { + $baseClass = config('ddd.base_model'); + $baseClassName = class_basename($baseClass); + + return [ + 'extends' => filled($baseClass) ? " extends {$baseClassName}" : '', + 'baseClassImport' => filled($baseClass) ? "use {$baseClass};" : '', + ]; + } + public function handle() + { + $this->createBaseModelIfNeeded(); + + parent::handle(); + + if ($this->option('factory')) { + $this->createFactory(); + } + } + + protected function createBaseModelIfNeeded() { $baseModel = config('ddd.base_model'); - $parts = str($baseModel)->explode('\\'); - $baseModelName = $parts->last(); + if (class_exists($baseModel)) { + return; + } + + $this->warn("Configured base model {$baseModel} doesn't exist."); + + // If the base model is out of scope, we won't attempt to create it + // because we don't want to interfere with external folders. + $allowedNamespacePrefixes = [ + $this->rootNamespace(), + ]; + + if (! str($baseModel)->startsWith($allowedNamespacePrefixes)) { + return; + } + + $domain = DomainResolver::guessDomainFromClass($baseModel); + + if (! $domain) { + return; + } + + $baseModelName = class_basename($baseModel); $baseModelPath = $this->getPath($baseModel); if (! file_exists($baseModelPath)) { - $this->warn("Base model {$baseModel} doesn't exist, generating..."); + $this->info("Generating {$baseModel}..."); $this->call(MakeBaseModel::class, [ - 'domain' => 'Shared', + 'domain' => $domain, 'name' => $baseModelName, ]); } - - parent::handle(); - - if ($this->option('factory')) { - $this->createFactory(); - } } protected function createFactory() diff --git a/src/Support/DomainResolver.php b/src/Support/DomainResolver.php index 43e075d..04efe97 100644 --- a/src/Support/DomainResolver.php +++ b/src/Support/DomainResolver.php @@ -4,7 +4,20 @@ class DomainResolver { - public static function fromModelClass(string $modelClass) + public static function guessDomainFromClass(string $class): ?string { + $domainNamespace = basename(config('ddd.paths.domains')).'\\'; + + if (! str($class)->startsWith($domainNamespace)) { + // Not a domain model + return null; + } + + $domain = str($class) + ->after($domainNamespace) + ->before('\\') + ->toString(); + + return $domain; } } diff --git a/stubs/model.php.stub b/stubs/model.php.stub index db53f28..e775c33 100644 --- a/stubs/model.php.stub +++ b/stubs/model.php.stub @@ -2,10 +2,10 @@ namespace {{ namespace }}; -use {{ rootNamespace }}\Shared\Models\BaseModel; +{{ baseClassImport }} use Illuminate\Database\Eloquent\SoftDeletes; -class {{ class }} extends BaseModel +class {{ class }}{{ extends }} { use SoftDeletes; diff --git a/tests/Generator/MakeModelTest.php b/tests/Generator/MakeModelTest.php index 55628ae..5884d66 100644 --- a/tests/Generator/MakeModelTest.php +++ b/tests/Generator/MakeModelTest.php @@ -106,10 +106,12 @@ expect(file_exists($expectedModelPath))->toBeTrue(); })->with('makeModelInputs'); -it('generates the base model if needed', function () { +it('generates the base model when possible', function ($baseModelClass, $baseModelPath) { $modelName = Str::studly(fake()->word()); $domain = Str::studly(fake()->word()); + Config::set('ddd.base_model', $baseModelClass); + $expectedModelPath = base_path(implode('/', [ config('ddd.paths.domains'), $domain, @@ -117,29 +119,72 @@ "{$modelName}.php", ])); + $expectedModelClass = implode('\\', [ + basename(config('ddd.paths.domains')), + $domain, + config('ddd.namespaces.models'), + $modelName, + ]); + if (file_exists($expectedModelPath)) { unlink($expectedModelPath); } expect(file_exists($expectedModelPath))->toBeFalse(); - // This currently only tests for the default base model - $expectedBaseModelPath = base_path(config('ddd.paths.domains').'/Shared/Models/BaseModel.php'); + $expectedBaseModelPath = base_path($baseModelPath); if (file_exists($expectedBaseModelPath)) { unlink($expectedBaseModelPath); } - // Todo: should bypass base model creation if - // a custom base model is being used. - // $baseModel = config('ddd.base_model'); + expect(class_exists($baseModelClass))->toBeFalse(); - expect(file_exists($expectedBaseModelPath))->toBeFalse(); + expect(file_exists($expectedBaseModelPath))->toBeFalse("{$baseModelPath} expected not to exist."); Artisan::call("ddd:model {$domain} {$modelName}"); - expect(file_exists($expectedBaseModelPath))->toBeTrue(); -}); + expect(file_exists($expectedBaseModelPath))->toBeTrue("Expecting base model file to be generated at {$baseModelPath}"); + + // Not able to properly assert the following class_exists checks under the testing environment + // expect(class_exists($expectedModelClass))->toBeTrue("Expecting model class {$expectedModelClass} to exist"); + // expect(class_exists($baseModelClass))->toBeTrue("Expecting base model class {$baseModelClass} to exist"); +})->with([ + ['Domain\Shared\Models\CustomBaseModel', 'src/Domain/Shared/Models/CustomBaseModel.php'], + ['Domain\Core\Models\CustomBaseModel', 'src/Domain/Core/Models/CustomBaseModel.php'], +]); + +it('will not generate a base model if the configured base model is out of scope', function ($baseModel) { + Config::set('ddd.base_model', $baseModel); + + expect(class_exists($baseModel))->toBeFalse(); + + Artisan::call('ddd:model Fruits Lemon'); + + expect(Artisan::output()) + ->toContain("Configured base model {$baseModel} doesn't exist.") + ->not->toContain("Generating {$baseModel}"); + + expect(class_exists($baseModel))->toBeFalse(); +})->with([ + ['Illuminate\Database\Eloquent\NonExistentModel'], + ['OtherVendor\OtherPackage\Models\NonExistentModel'], +]); + +it('skips base model creation if configured base model already exists', function ($baseModel) { + Config::set('ddd.base_model', $baseModel); + + expect(class_exists($baseModel))->toBeTrue(); + + Artisan::call('ddd:model Fruits Lemon'); + + expect(Artisan::output()) + ->not->toContain("Configured base model {$baseModel} doesn't exist.") + ->not->toContain("Generating {$baseModel}"); +})->with([ + ['Illuminate\Database\Eloquent\Model'], + ['Lunarstorm\LaravelDDD\Models\DomainModel'], +]); it('shows meaningful hints when prompting for missing input', function () { $this->artisan('ddd:model') diff --git a/tests/Model/FactoryTest.php b/tests/Model/FactoryTest.php index 9b33e11..afe9368 100644 --- a/tests/Model/FactoryTest.php +++ b/tests/Model/FactoryTest.php @@ -2,6 +2,7 @@ use Illuminate\Database\Eloquent\Factories\Factory; use Illuminate\Support\Facades\Artisan; +use Illuminate\Support\Facades\Config; use Lunarstorm\LaravelDDD\Factories\DomainFactory; it('can resolve the factory name of a domain model', function ($modelClass, $expectedFactoryClass) { @@ -13,6 +14,7 @@ ]); it('can instantiate a domain model factory', function ($domainParameter, $modelName, $modelClass) { + Config::set('ddd.base_model', 'Lunarstorm\LaravelDDD\Models\DomainModel'); Artisan::call("ddd:model -f {$domainParameter} {$modelName}"); expect(class_exists($modelClass))->toBeTrue(); expect($modelClass::factory())->toBeInstanceOf(Factory::class); diff --git a/tests/TestCase.php b/tests/TestCase.php index 44aaa10..9788b7f 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -22,6 +22,7 @@ protected function setUp(): void // Reset the domain namespace Arr::forget($data, ['autoload', 'psr-4', 'Domains\\']); + Arr::forget($data, ['autoload', 'psr-4', 'Domain\\']); // Set up the essential app namespaces data_set($data, ['autoload', 'psr-4', 'App\\'], 'vendor/orchestra/testbench-core/laravel/app'); @@ -36,7 +37,9 @@ protected function setUp(): void fn (string $modelName) => 'Lunarstorm\\LaravelDDD\\Database\\Factories\\'.class_basename($modelName).'Factory' ); - $this->beforeApplicationDestroyed(fn () => $this->cleanFilesAndFolders()); + $this->beforeApplicationDestroyed(function () { + $this->cleanFilesAndFolders(); + }); } protected function getPackageProviders($app)