Merge pull request #152 from ysfks/latest

Add condition to skip retrieving models that are not cascadable

.gitignore

@@ -19,4 +19,6 @@ tests/logs
 build/
 
 # PhpStorm
-.idea
+.idea
+
+.phpunit.cache

src/Listeners/CascadeQueryListener.php

@@ -3,6 +3,7 @@
 namespace Askedio\SoftCascade\Listeners;
 
 use Askedio\SoftCascade\QueryBuilderSoftCascade;
+use Askedio\SoftCascade\Traits\ChecksCascading;
 use BadMethodCallException;
 use Illuminate\Database\Connection;
 use Illuminate\Database\Eloquent\SoftDeletingScope;
@@ -11,6 +12,7 @@
 
 class CascadeQueryListener
 {
+    use ChecksCascading;
     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->hasCascadingRelations($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']);
         }
     }

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\ChecksCascading;
 use BadMethodCallException;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Relations\BelongsToMany;
@@ -14,6 +15,8 @@
 
 class SoftCascade implements SoftCascadeable
 {
+    use ChecksCascading;
+
     protected $direction;
     protected $directionData;
     protected $availableActions = ['update', 'restrict'];
@@ -66,7 +69,7 @@ protected function run($models)
                 return;
             }
 
-            if (!$this->isCascadable($model)) {
+            if (!$this->hasCascadingRelations($model)) {
                 return;
             }
 
@@ -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.
      *

src/Traits/ChecksCascading.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace Askedio\SoftCascade\Traits;
+
+trait ChecksCascading
+{
+    /**
+     * Check if the model is enabled to cascade.
+     *
+     * @param \Illuminate\Database\Eloquent\Model $model
+     *
+     * @return bool
+     */
+    protected function hasCascadingRelations($model): bool
+    {
+        return method_exists($model, 'getSoftCascade');
+    }
+}

tests/App/Record.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace Askedio\Tests\App;
+
+use Illuminate\Database\Eloquent\Model;
+
+class Record extends Model
+{
+    public $primaryKey = null;
+
+    public $timestamps = false;
+
+    public $incrementing = false;
+
+    protected $table = 'records';
+
+    protected $fillable = ['record_id', 'time', 'value'];
+}

tests/App/database/migrations/2024_10_23_095215_create_records_table.php

@@ -0,0 +1,31 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+
+class CreateRecordsTable extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        DB::connection()->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');
+    }
+}

tests/IntegrationTest.php

@@ -11,10 +11,14 @@
 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 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()
     // {
     /*