Replace custom SQL in removeBuilds with Eloquent relationships (#2624)

This commit introduces the use of Eloquent relationships to remove the following types of shared records: * buildfailure * buildfailuredetails * coveragefile
Kitware · Dec 19, 2024 · 689fa2e · 689fa2e
1 parent 65fe376
commit 689fa2e
Show file tree

Hide file tree

Showing 6 changed files with 237 additions and 69 deletions.
diff --git a/app/Models/BuildUpdate.php b/app/Models/BuildUpdate.php
@@ -0,0 +1,57 @@
+<?php
+
+namespace App\Models;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsToMany;
+use Illuminate\Support\Carbon;
+
+/**
+ * @property int $id
+ * @property Carbon $starttime
+ * @property Carbon $endtime
+ * @property string $command
+ * @property string $type
+ * @property int $nfiles
+ * @property int $warnings
+ * @property string $revision
+ * @property string $priorrevision
+ * @property string $path
+ *
+ * @mixin Builder<BuildUpdate>
+ */
+class BuildUpdate extends Model
+{
+    protected $table = 'buildupdate';
+
+    public $timestamps = false;
+
+    protected $fillable = [
+        'starttime',
+        'endtime',
+        'command',
+        'type',
+        'nfiles',
+        'warnings',
+        'revision',
+        'priorrevision',
+        'path',
+    ];
+
+    protected $casts = [
+        'id' => 'integer',
+        'starttime' => 'datetime',
+        'endtime' => 'datetime',
+        'nfiles' => 'integer',
+        'warnings' => 'integer',
+    ];
+
+    /**
+     * @return BelongsToMany<Build>
+     */
+    public function builds(): BelongsToMany
+    {
+        return $this->belongsToMany(Build::class, 'build2update', 'updateid', 'buildid');
+    }
+}
diff --git a/app/Models/CoverageFile.php b/app/Models/CoverageFile.php
@@ -0,0 +1,40 @@
+<?php
+
+namespace App\Models;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\HasManyThrough;
+
+/**
+ * @property int $id
+ * @property string $fullpath
+ * @property string $file
+ * @property int $crc32
+ *
+ * @mixin Builder<CoverageFile>
+ */
+class CoverageFile extends Model
+{
+    protected $table = 'coveragefile';
+
+    public $timestamps = false;
+
+    protected $fillable = [
+        'fullpath',
+        'file',
+        'crc32',
+    ];
+
+    protected $casts = [
+        'crc32' => 'integer',
+    ];
+
+    /**
+     * @return HasManyThrough<Build>
+     */
+    public function builds(): HasManyThrough
+    {
+        return $this->hasManyThrough(Build::class, Coverage::class, 'fileid', 'id', 'id', 'buildid');
+    }
+}
diff --git a/app/Models/RichBuildAlert.php b/app/Models/RichBuildAlert.php
@@ -0,0 +1,53 @@
+<?php
+
+namespace App\Models;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
+use Illuminate\Database\Eloquent\Relations\HasOne;
+
+/**
+ *
+ * @property int $buildid
+ * @property int $detailsid
+ * @property string $sourcefile
+ * @property int $newstatus
+ *
+ * @mixin Builder<RichBuildAlert>
+ */
+class RichBuildAlert extends Model
+{
+    protected $table = 'buildfailure';
+
+    public $timestamps = false;
+
+    protected $fillable = [
+        'buildid',
+        'detailsid',
+        'sourcefile',
+        'newstatus',
+    ];
+
+    protected $casts = [
+        'buildid' => 'integer',
+        'detailsid' => 'integer',
+        'newstatus' => 'integer',
+    ];
+
+    /**
+     * @return BelongsTo<Build, self>
+     */
+    public function build(): BelongsTo
+    {
+        return $this->belongsTo(Build::class, 'buildid');
+    }
+
+    /**
+     * @return HasOne<RichBuildAlertDetails>
+     */
+    public function details(): HasOne
+    {
+        return $this->hasOne(RichBuildAlertDetails::class, 'id', 'detailsid');
+    }
+}
diff --git a/app/Models/RichBuildAlertDetails.php b/app/Models/RichBuildAlertDetails.php
@@ -0,0 +1,53 @@
+<?php
+
+namespace App\Models;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\HasManyThrough;
+
+/**
+ * @property int $id
+ * @property int $type
+ * @property string $stdoutput
+ * @property string $stderror
+ * @property string $exitcondition
+ * @property string $language
+ * @property string $targetname
+ * @property string $outputfile
+ * @property string $outputtype
+ * @property int $crc32
+ *
+ * @mixin Builder<RichBuildAlertDetails>
+ */
+class RichBuildAlertDetails extends Model
+{
+    protected $table = 'buildfailuredetails';
+
+    public $timestamps = false;
+
+    protected $fillable = [
+        'type',
+        'stdoutput',
+        'stderror',
+        'exitcondition',
+        'language',
+        'targetname',
+        'outputfile',
+        'outputtype',
+        'crc32',
+    ];
+
+    protected $casts = [
+        'type' => 'integer',
+        'crc32' => 'integer',
+    ];
+
+    /**
+     * @return HasManyThrough<Build>
+     */
+    public function builds(): HasManyThrough
+    {
+        return $this->hasManyThrough(Build::class, RichBuildAlert::class, 'detailsid', 'id', 'id', 'buildid');
+    }
+}
diff --git a/app/Utils/DatabaseCleanupUtils.php b/app/Utils/DatabaseCleanupUtils.php
@@ -6,8 +6,11 @@
 
 use App\Models\Build;
 use App\Models\BuildGroup;
+use App\Models\BuildUpdate;
 use App\Models\Configure;
+use App\Models\CoverageFile;
 use App\Models\Note;
+use App\Models\RichBuildAlertDetails;
 use CDash\Database;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Support\Facades\DB;
@@ -120,89 +123,51 @@ public static function removeBuild($buildid) : void
             $buildids[] = intval($b);
         }
 
-        $db = Database::getInstance();
-        $buildid_prepare_array = $db->createPreparedArray(count($buildids));
+        // Use Eloquent relationships to delete shared records that are only
+        // used by builds that are about to be deleted.
 
-        // Delete buildfailuredetails that are only used by builds that are being
-        // deleted.
-        DB::delete("
-            DELETE FROM buildfailuredetails WHERE id IN (
-                SELECT a.detailsid
-                FROM buildfailure AS a
-                LEFT JOIN buildfailure AS b ON (
-                    a.detailsid=b.detailsid
-                    AND b.buildid NOT IN $buildid_prepare_array
-                )
-                WHERE a.buildid IN $buildid_prepare_array
-                GROUP BY a.detailsid
-                HAVING count(b.detailsid)=0
-            )
-        ", array_merge($buildids, $buildids));
-
-        // Delete the configure if not shared.
+        // buildfailuredetails
+        RichBuildAlertDetails::whereHas('builds', function (Builder $query) use ($buildids) {
+            $query->whereIn('build.id', $buildids);
+        })
+        ->whereDoesntHave('builds', function (Builder $query) use ($buildids) {
+            $query->whereNotIn('build.id', $buildids);
+        })->delete();
+
+        // configure
         Configure::whereHas('builds', function (Builder $query) use ($buildids) {
             $query->whereIn('id', $buildids);
         })
         ->whereDoesntHave('builds', function (Builder $query) use ($buildids) {
             $query->whereNotIn('id', $buildids);
         })->delete();
 
-        // coverage files are kept unless they are shared
-        DB::delete("
-            DELETE FROM coveragefile
-            WHERE id IN (
-                SELECT f1.id
-                FROM (
-                    SELECT a.fileid AS id, COUNT(DISTINCT a.buildid) AS c
-                    FROM coverage a
-                    WHERE a.buildid IN $buildid_prepare_array
-                    GROUP BY a.fileid
-                 ) AS f1
-                INNER JOIN (
-                    SELECT b.fileid AS id, COUNT(DISTINCT b.buildid) AS c
-                    FROM coverage b
-                    INNER JOIN (
-                        SELECT fileid
-                        FROM coverage
-                        WHERE buildid IN $buildid_prepare_array
-                    ) AS d ON b.fileid = d.fileid
-                    GROUP BY b.fileid
-                ) AS f2 ON (f1.id = f2.id)
-                WHERE f1.c = f2.c
-            )
-        ", array_merge($buildids, $buildids));
-
-        // Delete notes if not shared.
+        // coveragefile
+        CoverageFile::whereHas('builds', function (Builder $query) use ($buildids) {
+            $query->whereIn('build.id', $buildids);
+        })
+        ->whereDoesntHave('builds', function (Builder $query) use ($buildids) {
+            $query->whereNotIn('build.id', $buildids);
+        })->delete();
+
+        // note
         Note::whereHas('builds', function (Builder $query) use ($buildids) {
             $query->whereIn('id', $buildids);
         })
         ->whereDoesntHave('builds', function (Builder $query) use ($buildids) {
             $query->whereNotIn('id', $buildids);
         })->delete();
 
-        // Delete the update if not shared
-        $build2update = DB::select("
-                            SELECT a.updateid
-                            FROM build2update AS a
-                            LEFT JOIN build2update AS b ON (
-                                a.updateid=b.updateid
-                                AND b.buildid NOT IN $buildid_prepare_array
-                            )
-                            WHERE a.buildid IN $buildid_prepare_array
-                            GROUP BY a.updateid
-                            HAVING count(b.updateid)=0
-                        ", array_merge($buildids, $buildids));
-
-        $updateids = [];
-        foreach ($build2update as $build2update_array) {
-            // Update is not shared we delete
-            $updateids[] = intval($build2update_array->updateid);
-        }
+        // buildupdate
+        BuildUpdate::whereHas('builds', function (Builder $query) use ($buildids) {
+            $query->whereIn('build.id', $buildids);
+        })
+        ->whereDoesntHave('builds', function (Builder $query) use ($buildids) {
+            $query->whereNotIn('build.id', $buildids);
+        })->delete();
 
-        if (count($updateids) > 0) {
-            $updateids_prepare_array = $db->createPreparedArray(count($updateids));
-            DB::delete("DELETE FROM buildupdate WHERE id IN $updateids_prepare_array", $updateids);
-        }
+        $db = Database::getInstance();
+        $buildid_prepare_array = $db->createPreparedArray(count($buildids));
 
         // Delete tests and testoutputs that are not shared.
         // First find all the tests and testoutputs from builds that are about to be deleted.

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -3842,12 +3842,12 @@ parameters:
 
 		-
 			message: "#^Dynamic call to static method Illuminate\\\\Database\\\\Eloquent\\\\Builder\\<Illuminate\\\\Database\\\\Eloquent\\\\Model\\>\\:\\:whereIn\\(\\)\\.$#"
-			count: 2
+			count: 5
 			path: app/Utils/DatabaseCleanupUtils.php
 
 		-
 			message: "#^Dynamic call to static method Illuminate\\\\Database\\\\Eloquent\\\\Builder\\<Illuminate\\\\Database\\\\Eloquent\\\\Model\\>\\:\\:whereNotIn\\(\\)\\.$#"
-			count: 2
+			count: 5
 			path: app/Utils/DatabaseCleanupUtils.php
 
 		-