Skip to content

Commit cd49a87

Browse files
williamjallenwilliamjallen
authored
Renormalize label-coverage relationship (#2950)
The `label2coveragefile` table implicitly relates the `coverage` and `label` tables via a three-way relationship, duplicating the role of the `coverage` table which relates the `build` and `coveragefile` tables. This PR fixes that by creating a new `label2coverage` table which appropriately relates labels to coverage results.
1 parent 8cb128f commit cd49a87

File tree

12 files changed

+91
-96
lines changed

12 files changed

+91
-96
lines changed

‎app/Http/Controllers/CoverageController.php‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -232,11 +232,12 @@ public function viewCoverage(): View|RedirectResponse
232232
$labels = [];
233233

234234
$covlabels = $db->executePrepared('
235-
SELECT DISTINCT id, text
236-
FROM label, label2coveragefile
235+
SELECT DISTINCT label.id, label.text
236+
FROM label, label2coverage, coverage
237237
WHERE
238-
label.id=label2coveragefile.labelid
239-
AND label2coveragefile.buildid=?
238+
label.id=label2coverage.labelid
239+
AND label2coverage.coverageid=coverage.id
240+
AND coverage.buildid=?
240241
', [intval($this->build->Id)]);
241242
foreach ($covlabels as $row) {
242243
$labels[$row['id']] = $row['text'];
@@ -253,12 +254,11 @@ public function viewCoverage(): View|RedirectResponse
253254
SELECT
254255
SUM(loctested) AS loctested,
255256
SUM(locuntested) AS locuntested
256-
FROM label2coveragefile, coverage
257+
FROM label2coverage, coverage
257258
WHERE
258-
label2coveragefile.labelid=?
259-
AND label2coveragefile.buildid=?
260-
AND coverage.buildid=label2coveragefile.buildid
261-
AND coverage.fileid=label2coveragefile.coveragefileid
259+
label2coverage.labelid=?
260+
AND coverage.buildid=?
261+
AND coverage.id=label2coverage.coverageid
262262
', [intval($id), intval($this->build->Id)]);
263263

264264
$loctested = intval($row['loctested']);
@@ -1242,11 +1242,13 @@ public function ajaxGetViewCoverage(): JsonResponse
12421242
SELECT text
12431243
FROM
12441244
label,
1245-
label2coveragefile
1245+
label2coverage,
1246+
coverage
12461247
WHERE
1247-
label.id=label2coveragefile.labelid
1248-
AND label2coveragefile.coveragefileid=?
1249-
AND label2coveragefile.buildid=?
1248+
label.id=label2coverage.labelid
1249+
AND label2coverage.coverageid=coverage.id
1250+
AND coverage.fileid=?
1251+
AND coverage.buildid=?
12501252
ORDER BY text ASC
12511253
', [intval($fileid), $this->build->Id]);
12521254
foreach ($coveragelabels as $coveragelabels_array) {

‎app/Models/Coverage.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Illuminate\Database\Eloquent\Builder;
66
use Illuminate\Database\Eloquent\Model;
77
use Illuminate\Database\Eloquent\Relations\BelongsTo;
8+
use Illuminate\Database\Eloquent\Relations\BelongsToMany;
89
use Illuminate\Database\Eloquent\Relations\HasOne;
910

1011
/**
@@ -46,6 +47,14 @@ public function build(): BelongsTo
4647
return $this->belongsTo(Build::class, 'buildid');
4748
}
4849

50+
/**
51+
* @return BelongsToMany<Label, $this>
52+
*/
53+
public function labels(): BelongsToMany
54+
{
55+
return $this->belongsToMany(Label::class, 'label2coverage', 'coverageid', 'labelid');
56+
}
57+
4958
/**
5059
* @return HasOne<CoverageFile, $this>
5160
*/

‎app/cdash/app/Model/Build.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1493,8 +1493,9 @@ public function GetLabels($labelarray = []): array|false
14931493
if (empty($labelarray) || isset($labelarray['coverage']['errors'])) {
14941494
$sql .=
14951495
' OR label.id IN
1496-
(SELECT labelid AS id FROM label2coveragefile
1497-
WHERE label2coveragefile.buildid = :buildid)';
1496+
(SELECT labelid AS id FROM label2coverage
1497+
INNER JOIN coverage ON (coverage.id=label2coverage.coverageid)
1498+
WHERE coverage.buildid = :buildid)';
14981499
}
14991500
if (empty($labelarray) || isset($labelarray['build']['errors'])) {
15001501
$sql .=

‎app/cdash/app/Model/Coverage.php‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
namespace CDash\Model;
1919

2020
use App\Models\Coverage as EloquentCoverage;
21-
use Illuminate\Support\Facades\Log;
2221

2322
/**
2423
* Coverage class. Used by CoverageSummary
@@ -46,34 +45,9 @@ public function AddLabel($label)
4645
$this->Labels = [];
4746
}
4847

49-
$label->CoverageFileId = $this->CoverageFile->Id;
50-
$label->CoverageFileBuildId = (int) $this->BuildId;
5148
$this->Labels[] = $label;
5249
}
5350

54-
/** Put labels for coverage */
55-
public function InsertLabelAssociations($buildid)
56-
{
57-
if ($buildid
58-
&& isset($this->CoverageFile)
59-
&& $this->CoverageFile->Id
60-
) {
61-
if (empty($this->Labels)) {
62-
return;
63-
}
64-
65-
foreach ($this->Labels as $label) {
66-
$label->CoverageFileId = $this->CoverageFile->Id;
67-
$label->CoverageFileBuildId = (int) $buildid;
68-
$label->Insert();
69-
}
70-
} else {
71-
Log::error('No buildid or coveragefile', [
72-
'function' => 'Coverage::InsertLabelAssociations',
73-
]);
74-
}
75-
}
76-
7751
/** Return true if this build already has coverage for this file,
7852
* false otherwise.
7953
**/

‎app/cdash/app/Model/CoverageFile.php‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -90,24 +90,6 @@ public function Update($buildid)
9090
'fileid' => $this->Id,
9191
]);
9292

93-
// Similarly update any labels if necessary.
94-
$stmt = $this->PDO->prepare(
95-
'SELECT COUNT(*) AS c FROM label2coveragefile
96-
WHERE buildid=:buildid AND coveragefileid=:prevfileid');
97-
$stmt->bindParam(':buildid', $buildid);
98-
$stmt->bindParam(':prevfileid', $prevfileid);
99-
pdo_execute($stmt);
100-
$count_labels_row = $stmt->fetch(PDO::FETCH_ASSOC);
101-
if ($count_labels_row['c'] > 0) {
102-
$stmt = $this->PDO->prepare(
103-
'UPDATE label2coveragefile SET coveragefileid=:fileid
104-
WHERE buildid=:buildid AND coveragefileid=:prevfileid');
105-
$stmt->bindParam(':fileid', $this->Id);
106-
$stmt->bindParam(':buildid', $buildid);
107-
$stmt->bindParam(':prevfileid', $prevfileid);
108-
pdo_execute($stmt);
109-
}
110-
11193
// Remove the file if the crc32 is NULL
11294
EloquentCoverageFile::where([
11395
'id' => $prevfileid,

‎app/cdash/app/Model/CoverageSummary.php‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919

2020
use App\Models\Coverage;
2121
use App\Models\CoverageFile;
22+
use App\Models\Label;
2223
use CDash\Database;
24+
use Exception;
2325
use Illuminate\Support\Facades\DB;
2426

2527
class CoverageSummary
@@ -142,18 +144,20 @@ public function Insert($append = false): bool
142144

143145
$existing_row_updated = false;
144146

147+
$eloquent_coverage = null;
148+
145149
// TODO: replace the following two conditionals with a single call to updateOrCreate()
146150
if ($append) {
147151
// UPDATE (instead of INSERT) if this coverage already
148152
// exists.
149-
$existing_row_updated = DB::transaction(function () use ($coverage, $covered, $loctested, $locuntested, $branchestested, $branchesuntested, $functionstested, $functionsuntested) {
150-
$existing_coverage_row = Coverage::firstWhere([
153+
$existing_row_updated = DB::transaction(function () use ($coverage, $covered, $loctested, $locuntested, $branchestested, $branchesuntested, $functionstested, $functionsuntested, &$eloquent_coverage) {
154+
$eloquent_coverage = Coverage::firstWhere([
151155
'buildid' => $this->BuildId,
152156
'fileid' => $coverage->CoverageFile->Id,
153157
]);
154158

155-
if ($existing_coverage_row !== null) {
156-
$existing_coverage_row->update([
159+
if ($eloquent_coverage !== null) {
160+
$eloquent_coverage->update([
157161
'covered' => $covered,
158162
'loctested' => $loctested,
159163
'locuntested' => $locuntested,
@@ -168,7 +172,7 @@ public function Insert($append = false): bool
168172
});
169173
}
170174
if (!$existing_row_updated) {
171-
Coverage::create([
175+
$eloquent_coverage = Coverage::create([
172176
'buildid' => $this->BuildId,
173177
'fileid' => $fileid,
174178
'covered' => $covered,
@@ -180,11 +184,15 @@ public function Insert($append = false): bool
180184
'functionsuntested' => $functionsuntested,
181185
]);
182186
}
183-
}
184187

185-
// Add labels
186-
foreach ($this->Coverages as &$coverage) {
187-
$coverage->InsertLabelAssociations($this->BuildId);
188+
// This case should never happen, but we check just in case to make PHPStan happy
189+
if ($eloquent_coverage === null) {
190+
throw new Exception('Invalid state: coverage model does not exist.');
191+
}
192+
193+
foreach ($coverage->Labels ?? [] as $label) {
194+
$eloquent_coverage->labels()->syncWithoutDetaching(Label::firstOrCreate(['text' => $label->Text]));
195+
}
188196
}
189197
}
190198

‎app/cdash/app/Model/Label.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,6 @@ public function Insert()
119119

120120
$this->InsertAssociation('label2buildfailure', 'buildfailureid', intval($this->BuildFailureId));
121121

122-
$this->InsertAssociation('label2coveragefile', 'buildid',
123-
$this->CoverageFileBuildId, 'coveragefileid', intval($this->CoverageFileId));
124-
125122
$this->InsertAssociation('label2dynamicanalysis', 'dynamicanalysisid', intval($this->DynamicAnalysisId));
126123

127124
$this->Test?->labels()->syncWithoutDetaching([$this->Id]);

‎app/cdash/app/Model/Project.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -838,9 +838,10 @@ public function GetLabels($days): array|false
838838
AND build.starttime>?
839839
) UNION (
840840
SELECT labelid AS id
841-
FROM build, label2coveragefile
841+
FROM build, label2coverage, coverage
842842
WHERE
843-
label2coveragefile.buildid=build.id
843+
label2coverage.coverageid=coverage.id
844+
AND coverage.buildid=build.id
844845
AND build.projectid=?
845846
AND build.starttime>?
846847
) UNION (

‎app/cdash/include/filterdataFunctions.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ public function getSqlField($field)
506506
break;
507507

508508
case 'labels':
509-
$sql_field = "(SELECT $this->TextConcat AS labels FROM (SELECT label.text, coverage.fileid, coverage.buildid FROM label, label2coveragefile, coverage WHERE label2coveragefile.labelid=label.id AND label2coveragefile.buildid=coverage.buildid AND label2coveragefile.coveragefileid=coverage.fileid) AS filelabels WHERE fileid=c.fileid AND buildid=c.buildid)";
509+
$sql_field = "(SELECT $this->TextConcat AS labels FROM (SELECT label.text, coverage.fileid, coverage.buildid FROM label, label2coverage, coverage WHERE label2coverage.labelid=label.id AND label2coverage.coverageid=coverage.id) AS filelabels WHERE fileid=c.fileid AND buildid=c.buildid)";
510510

511511
break;
512512

‎app/cdash/tests/test_removebuilds.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ public function testBuildRemovalWorksAsExpected(): void
495495
$this->verify_get_rows('label2build', 'labelid', 'buildid', '=', $build->Id, 1);
496496
$this->verify('label', 'id', '=', $labelid, 1);
497497
$this->verify('label2buildfailure', 'labelid', '=', $labelid, 2);
498-
$this->verify('label2coveragefile', 'labelid', '=', $labelid, 3);
498+
$this->verify('label2coverage', 'labelid', '=', $labelid, 3);
499499
$this->verify('label2dynamicanalysis', 'labelid', '=', $labelid, 1);
500500
$this->verify('label2test', 'labelid', '=', $labelid, 3);
501501

@@ -533,7 +533,7 @@ public function testBuildRemovalWorksAsExpected(): void
533533
$this->verify('image', 'id', 'IN', $imgids, 1, $extra_msg);
534534
$this->verify('label2build', 'buildid', '=', $build->Id, 0, $extra_msg);
535535
$this->verify('label2buildfailure', 'labelid', '=', $labelid, 1, $extra_msg);
536-
$this->verify('label2coveragefile', 'labelid', '=', $labelid, 1, $extra_msg);
536+
$this->verify('label2coverage', 'labelid', '=', $labelid, 1, $extra_msg);
537537
$this->verify('label2dynamicanalysis', 'labelid', '=', $labelid, 0, $extra_msg);
538538
$this->verify('label2test', 'labelid', '=', $labelid, 1, $extra_msg);
539539
$this->verify('note', 'id', 'IN', $noteids, 1, $extra_msg);
@@ -576,7 +576,7 @@ public function testBuildRemovalWorksAsExpected(): void
576576
$this->verify('image', 'id', 'IN', $imgids, 0, $extra_msg);
577577
$this->verify('label2build', 'buildid', '=', $existing_build->Id, 0, $extra_msg);
578578
$this->verify('label2buildfailure', 'labelid', '=', $labelid, 0, $extra_msg);
579-
$this->verify('label2coveragefile', 'labelid', '=', $labelid, 0, $extra_msg);
579+
$this->verify('label2coverage', 'labelid', '=', $labelid, 0, $extra_msg);
580580
$this->verify('label2dynamicanalysis', 'labelid', '=', $labelid, 0, $extra_msg);
581581
$this->verify('label2test', 'labelid', '=', $labelid, 0, $extra_msg);
582582
$this->verify('note', 'id', 'IN', $noteids, 0, $extra_msg);

0 commit comments

Comments
 (0)