From 0a4cba42879738220b70f10fbc2de2e8880a8cbb Mon Sep 17 00:00:00 2001 From: ysf ks Date: Wed, 23 Oct 2024 13:16:27 +0200 Subject: [PATCH 1/5] Add condition to skip selecting model when model is not cascadable or model primary key is null, Add test case --- .gitignore | 4 ++- src/Listeners/CascadeQueryListener.php | 11 ++++++- src/SoftCascade.php | 15 ++------- src/Traits/Cascadable.php | 18 +++++++++++ tests/App/Record.php | 18 +++++++++++ ...2024_10_23_095215_create_records_table.php | 31 +++++++++++++++++++ tests/IntegrationTest.php | 29 +++++++++++++++++ 7 files changed, 112 insertions(+), 14 deletions(-) create mode 100644 src/Traits/Cascadable.php create mode 100644 tests/App/Record.php create mode 100644 tests/App/database/migrations/2024_10_23_095215_create_records_table.php diff --git a/.gitignore b/.gitignore index 39ea6ed..c5f1ddb 100644 --- a/.gitignore +++ b/.gitignore @@ -19,4 +19,6 @@ tests/logs build/ # PhpStorm -.idea \ No newline at end of file +.idea + +.phpunit.cache \ No newline at end of file diff --git a/src/Listeners/CascadeQueryListener.php b/src/Listeners/CascadeQueryListener.php index c4f5ead..84399bc 100644 --- a/src/Listeners/CascadeQueryListener.php +++ b/src/Listeners/CascadeQueryListener.php @@ -3,6 +3,7 @@ namespace Askedio\SoftCascade\Listeners; use Askedio\SoftCascade\QueryBuilderSoftCascade; +use Askedio\SoftCascade\Traits\Cascadable; use BadMethodCallException; use Illuminate\Database\Connection; use Illuminate\Database\Eloquent\SoftDeletingScope; @@ -11,6 +12,7 @@ class CascadeQueryListener { + use Cascadable; const EVENT = QueryExecuted::class; /** @@ -82,7 +84,14 @@ public function handle(): void // otherwise, we can just skip it } - $model = $builder->get([$builder->getModel()->getKeyName()]); + $keyName = $builder->getModel()->getKeyName(); + + if (!$this->isCascadable($builder->getModel()) or $keyName === null) { + // If model doesn't have any primary key, there will be no relations + return; + } + + $model = $builder->get([$keyName]); (new QueryBuilderSoftCascade())->cascade($model, $event['direction'], $event['directionData']); } } diff --git a/src/SoftCascade.php b/src/SoftCascade.php index dbfff5d..f70b53e 100644 --- a/src/SoftCascade.php +++ b/src/SoftCascade.php @@ -6,6 +6,7 @@ use Askedio\SoftCascade\Exceptions\SoftCascadeLogicException; use Askedio\SoftCascade\Exceptions\SoftCascadeNonExistentRelationActionException; use Askedio\SoftCascade\Exceptions\SoftCascadeRestrictedException; +use Askedio\SoftCascade\Traits\Cascadable; use BadMethodCallException; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\BelongsToMany; @@ -14,6 +15,8 @@ class SoftCascade implements SoftCascadeable { + use Cascadable; + protected $direction; protected $directionData; protected $availableActions = ['update', 'restrict']; @@ -237,18 +240,6 @@ protected function validateRelation($model, $relation) } } - /** - * Check if the model is enabled to cascade. - * - * @param Illuminate\Database\Eloquent\Model $model - * - * @return bool - */ - protected function isCascadable($model) - { - return method_exists($model, 'getSoftCascade'); - } - /** * Affected rows if we do execute. * diff --git a/src/Traits/Cascadable.php b/src/Traits/Cascadable.php new file mode 100644 index 0000000..accd5d7 --- /dev/null +++ b/src/Traits/Cascadable.php @@ -0,0 +1,18 @@ +getSchemaBuilder()->create('records', function (Blueprint $table) { + $table->string('record_id'); + $table->float('value'); + $table->timestamp('time'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + DB::connection()->getSchemaBuilder()->dropIfExists('records'); + } +} diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index 11eddbc..1346b9e 100755 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -15,6 +15,10 @@ use Askedio\Tests\App\RoleWriter; use Askedio\Tests\App\User; use Askedio\Tests\App\Video; +use Askedio\Tests\App\Record; +use Illuminate\Database\Events\QueryExecuted; +use Illuminate\Support\Facades\Event; +use Illuminate\Support\Str; /** * TO-DO: Need better testing. @@ -312,6 +316,31 @@ public function testInexistentRestrictedAction() BadRelationAction::first()->delete(); } + public function testSkipCascadingModelsWithoutPrimaryKeys() + { + $testSelect = false; + + Event::listen(QueryExecuted::class, function (QueryExecuted $event) use (&$testSelect) { + if ($testSelect && !Str::of($event->sql)->contains('delete', true)) { + $this->fail('Model shouldn\'t have been retrieved when it is not cascadable or its primary key is null'); + } + }); + + $record = Record::create([ + 'record_id' => 'b61bc6dd-4cee-4fe2-8aed-d8eac5920532', + 'time' => now()->subMonth(), + 'value' => 134.12 + ]); + + $testSelect = true; + + Record::query()->where('time', '<', now())->delete(); + + $testSelect = false; + + $this->assertDatabaseMissing('records', ['record_id' => $record->record_id]); + } + // public function testNotCascadable() // { /* From 26e3f8063bd55257d9d063743f52c8b2fa188179 Mon Sep 17 00:00:00 2001 From: ysf ks Date: Wed, 23 Oct 2024 13:37:54 +0200 Subject: [PATCH 2/5] Rename trait and the method according to its purpose --- src/Listeners/CascadeQueryListener.php | 6 +++--- src/SoftCascade.php | 6 +++--- src/Traits/{Cascadable.php => ChecksCascading.php} | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) rename src/Traits/{Cascadable.php => ChecksCascading.php} (77%) diff --git a/src/Listeners/CascadeQueryListener.php b/src/Listeners/CascadeQueryListener.php index 84399bc..d6a9a5c 100644 --- a/src/Listeners/CascadeQueryListener.php +++ b/src/Listeners/CascadeQueryListener.php @@ -3,7 +3,7 @@ namespace Askedio\SoftCascade\Listeners; use Askedio\SoftCascade\QueryBuilderSoftCascade; -use Askedio\SoftCascade\Traits\Cascadable; +use Askedio\SoftCascade\Traits\ChecksCascading; use BadMethodCallException; use Illuminate\Database\Connection; use Illuminate\Database\Eloquent\SoftDeletingScope; @@ -12,7 +12,7 @@ class CascadeQueryListener { - use Cascadable; + use ChecksCascading; const EVENT = QueryExecuted::class; /** @@ -86,7 +86,7 @@ public function handle(): void $keyName = $builder->getModel()->getKeyName(); - if (!$this->isCascadable($builder->getModel()) or $keyName === null) { + if (!$this->hasCascadingRelations($builder->getModel()) or $keyName === null) { // If model doesn't have any primary key, there will be no relations return; } diff --git a/src/SoftCascade.php b/src/SoftCascade.php index f70b53e..434ca09 100644 --- a/src/SoftCascade.php +++ b/src/SoftCascade.php @@ -6,7 +6,7 @@ use Askedio\SoftCascade\Exceptions\SoftCascadeLogicException; use Askedio\SoftCascade\Exceptions\SoftCascadeNonExistentRelationActionException; use Askedio\SoftCascade\Exceptions\SoftCascadeRestrictedException; -use Askedio\SoftCascade\Traits\Cascadable; +use Askedio\SoftCascade\Traits\ChecksCascading; use BadMethodCallException; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\BelongsToMany; @@ -15,7 +15,7 @@ class SoftCascade implements SoftCascadeable { - use Cascadable; + use ChecksCascading; protected $direction; protected $directionData; @@ -69,7 +69,7 @@ protected function run($models) return; } - if (!$this->isCascadable($model)) { + if (!$this->hasCascadingRelations($model)) { return; } diff --git a/src/Traits/Cascadable.php b/src/Traits/ChecksCascading.php similarity index 77% rename from src/Traits/Cascadable.php rename to src/Traits/ChecksCascading.php index accd5d7..816df87 100644 --- a/src/Traits/Cascadable.php +++ b/src/Traits/ChecksCascading.php @@ -2,7 +2,7 @@ namespace Askedio\SoftCascade\Traits; -trait Cascadable +trait ChecksCascading { /** * Check if the model is enabled to cascade. @@ -11,7 +11,7 @@ trait Cascadable * * @return bool */ - protected function isCascadable($model): bool + protected function hasCascadingRelations($model): bool { return method_exists($model, 'getSoftCascade'); } From 59873986e99fb68329048ba9ec30f951bb7c0a78 Mon Sep 17 00:00:00 2001 From: ysf ks Date: Wed, 23 Oct 2024 14:09:57 +0200 Subject: [PATCH 3/5] Fix style errors --- src/Traits/ChecksCascading.php | 1 + tests/IntegrationTest.php | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Traits/ChecksCascading.php b/src/Traits/ChecksCascading.php index 816df87..dfffe2f 100644 --- a/src/Traits/ChecksCascading.php +++ b/src/Traits/ChecksCascading.php @@ -15,4 +15,5 @@ protected function hasCascadingRelations($model): bool { return method_exists($model, 'getSoftCascade'); } + } \ No newline at end of file diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index 1346b9e..01212b1 100755 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -11,11 +11,11 @@ use Askedio\Tests\App\Languages; use Askedio\Tests\App\Post; use Askedio\Tests\App\Profiles; +use Askedio\Tests\App\Record; use Askedio\Tests\App\RoleReader; use Askedio\Tests\App\RoleWriter; use Askedio\Tests\App\User; use Askedio\Tests\App\Video; -use Askedio\Tests\App\Record; use Illuminate\Database\Events\QueryExecuted; use Illuminate\Support\Facades\Event; use Illuminate\Support\Str; @@ -328,8 +328,8 @@ public function testSkipCascadingModelsWithoutPrimaryKeys() $record = Record::create([ 'record_id' => 'b61bc6dd-4cee-4fe2-8aed-d8eac5920532', - 'time' => now()->subMonth(), - 'value' => 134.12 + 'time' => now()->subMonth(), + 'value' => 134.12, ]); $testSelect = true; From 8e49a0885492b6f5e90e21999ed8d1876c50670c Mon Sep 17 00:00:00 2001 From: ysf ks Date: Wed, 23 Oct 2024 14:11:08 +0200 Subject: [PATCH 4/5] Remove extra space from trait for style errors --- src/Traits/ChecksCascading.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Traits/ChecksCascading.php b/src/Traits/ChecksCascading.php index dfffe2f..816df87 100644 --- a/src/Traits/ChecksCascading.php +++ b/src/Traits/ChecksCascading.php @@ -15,5 +15,4 @@ protected function hasCascadingRelations($model): bool { return method_exists($model, 'getSoftCascade'); } - } \ No newline at end of file From feadc1565569e1364ab27c63f4a3cf1a2078d25a Mon Sep 17 00:00:00 2001 From: ysf ks Date: Wed, 23 Oct 2024 14:14:23 +0200 Subject: [PATCH 5/5] Fix styling error in ChecksCascading trait --- src/Traits/ChecksCascading.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Traits/ChecksCascading.php b/src/Traits/ChecksCascading.php index 816df87..5246e66 100644 --- a/src/Traits/ChecksCascading.php +++ b/src/Traits/ChecksCascading.php @@ -15,4 +15,4 @@ protected function hasCascadingRelations($model): bool { return method_exists($model, 'getSoftCascade'); } -} \ No newline at end of file +}