Skip to content

Commit

Permalink
Refactor testGraph.php API endpoint (#1890)
Browse files Browse the repository at this point in the history
This PR continues our ongoing effort to modernize the codebase by moving
the legacy API endpoint files to Laravel controllers.
  • Loading branch information
williamjallen committed Dec 27, 2023
1 parent 294c0fb commit 0ee31d1
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 58 deletions.
31 changes: 31 additions & 0 deletions app/Http/Controllers/TestController.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use App\Utils\PageTimer;
use CDash\Controller\Api\TestOverview as LegacyTestOverviewController;
use CDash\Controller\Api\TestDetails as LegacyTestDetailsController;
use CDash\Controller\Api\TestGraph as LegacyTestGraphController;
use CDash\Database;
use CDash\Model\Build;
use CDash\Model\Project;
Expand Down Expand Up @@ -497,4 +498,34 @@ public function apiTestSummary(): JsonResponse|StreamedResponse
$pageTimer->end($response);
return response()->json($response);
}

public function apiTestGraph(): JsonResponse
{
if (!request()->has('buildid')) {
abort(400, '"buildid" parameter is required.');
}
$buildid = (int) request()->input('buildid');
$build = new \CDash\Model\Build();
$build->FillFromId($buildid);
Gate::authorize('view-project', $build->GetProject());

$db = Database::getInstance();

$testid = request()->input('testid');
if (!is_numeric($testid)) {
abort(400, 'A valid test was not specified.');
}
$testid = (int) $testid;

$buildtest = BuildTest::where('buildid', '=', $buildid)
->where('testid', '=', $testid)
->first();
if ($buildtest === null) {
abort(404, 'test not found');
}

$controller = new LegacyTestGraphController($db, $buildtest);
$response = $controller->getResponse();
return response()->json(cast_data_for_JSON($response));
}
}
48 changes: 0 additions & 48 deletions app/cdash/public/api/v1/testGraph.php

This file was deleted.

13 changes: 8 additions & 5 deletions app/cdash/tests/test_testgraphpermissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,18 @@ public function testTestGraphPermissions()
$row = $stmt->fetch();
$testid = $row['testid'];

$result = $this->get($this->url . "/api/v1/testGraph.php?buildid={$this->build}&testid=$testid&type=time");
// Verify that we cannot access the graphs (because we're not logged in)
$response = json_decode($this->get($this->url . "/api/v1/testGraph.php?buildid={$this->build}&testid=$testid&type=time"), true);
if ($response['requirelogin'] != 1) {
$response = json_decode($result, true);
if ($response['error'] !== 'You do not have access to the requested project or the requested project does not exist.') {
$this->fail("Unauthorized case #1 fails");
}
$response = json_decode($this->get($this->url . "/api/v1/testGraph.php?buildid={$this->build}&testid=$testid&type=status"), true);
if ($response['requirelogin'] != 1) {
if ($response['error'] !== 'You do not have access to the requested project or the requested project does not exist.') {
$this->fail("Unauthorized case #2 fails");
}
$response = $this->get($this->url . "/ajax/showtestfailuregraph.php?testname=itkVectorSegmentationLevelSetFunctionTest1&projectid={$this->project}&starttime=1235350800");
if ($response !== 'You are not authorized to view this page.') {
if (str_contains($response, 'You are not authorized to view this page.')) {
$this->fail("Unauthorized case #3 fails");
}

Expand All @@ -70,8 +71,10 @@ public function testTestGraphPermissions()
if (array_key_exists('requirelogin', $response)) {
$this->fail("Authorized case #2 fails");
}

// A horrible hack to make sure a real page was rendered
$response = $this->get($this->url . "/ajax/showtestfailuregraph.php?testname=itkVectorSegmentationLevelSetFunctionTest1&projectid={$this->project}&starttime=1235350800");
if (strpos($response, '<br>') === false) {
if (!str_contains($response, '</script>')) {
$this->fail("Authorized case #3 fails");
}
}
Expand Down
5 changes: 0 additions & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -26746,11 +26746,6 @@ parameters:
count: 2
path: app/cdash/tests/test_testgraphpermissions.php

-
message: "#^Loose comparison via \"\\!\\=\" is not allowed\\.$#"
count: 2
path: app/cdash/tests/test_testgraphpermissions.php

-
message: "#^Method TestGraphPermissionsTestCase\\:\\:testTestGraphPermissions\\(\\) has no return type specified\\.$#"
count: 1
Expand Down
2 changes: 2 additions & 0 deletions routes/api.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@

Route::get('/v1/testSummary.php', 'TestController@apiTestSummary');

Route::get('/v1/testGraph.php', 'TestController@apiTestGraph');

Route::get('/v1/is_build_expected.php', 'BuildController@apiBuildExpected');

Route::get('/v1/buildUpdateGraph.php', 'BuildController@apiBuildUpdateGraph');
Expand Down

0 comments on commit 0ee31d1

Please sign in to comment.