infrastructure/blender-buildbot-www
1
0
Fork 2
Code Issues 4 Pull Requests 1 Projects Releases Wiki Activity

Add support for JSON listing #1

Merged
Francesco Siddi merged 7 commits from json-view into develop 2023-11-01 12:08:09 +01:00
Conversation 2 Commits 7 Files Changed 2 +57 -6
2 changed files with 57 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
source/BuildsRenderer.php
View File

@ -8,6 +8,31 @@ class BuildsRenderer {
$this->builds = $builds; $this->builds = $builds;
} }
public function renderJSON() {
/* Return a JSON formatted list of builds. */
$builds_json = array();
foreach ($this->builds as $build) {
$builds_json[] = array(
fsiddi marked this conversation as resolved Outdated
Sergey Sharybin commented 2023-11-01 10:10:24 +01:00
Outdated
Review
Copy Link

I didn't realize how it worked before. We should not be modifying builds from the renderer. And we should never inject fields which do not exist in the class.

Unfortunately, PHP does not provide anything to ensure const-correctness. But once the build and list of builds were constructed they should remain non-mutable.

More verbose but more correct is to create an explicit associative array with the fields which you're interested in:

$builds_json = array();

foreach ($this->builds as $build) {
  $builds_json[] = array(
    'url' => $this->getFileNameURL($build),
    'app' => $build->app,
    'version' => $build->version,
    'risk_id' => $build->risk_id,
    'branch' => $build->branch,
    'patch' => $build->patch,
    'hash' => $build->hash,
    'platform' => $build->platform,
    'architecture' => $build->architecture,
    'bitness' => $build->bitness,
    'file_mtime' => $build->file_mtime,
    'file_name' => $build->file_name,
    'file_size' => $build->file_size,
    'file_extension' => $build->file_extension,
    'release_cycle' => $build->release_cycle,
  );
}

echo(json_encode($builds_json, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
I didn't realize how it worked before. We should not be modifying builds from the renderer. And we should never inject fields which do not exist in the class. Unfortunately, PHP does not provide anything to ensure const-correctness. But once the build and list of builds were constructed they should remain non-mutable. More verbose but more correct is to create an explicit associative array with the fields which you're interested in: ``` $builds_json = array(); foreach ($this->builds as $build) { $builds_json[] = array( 'url' => $this->getFileNameURL($build), 'app' => $build->app, 'version' => $build->version, 'risk_id' => $build->risk_id, 'branch' => $build->branch, 'patch' => $build->patch, 'hash' => $build->hash, 'platform' => $build->platform, 'architecture' => $build->architecture, 'bitness' => $build->bitness, 'file_mtime' => $build->file_mtime, 'file_name' => $build->file_name, 'file_size' => $build->file_size, 'file_extension' => $build->file_extension, 'release_cycle' => $build->release_cycle, ); } echo(json_encode($builds_json, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); ```
'url' => $this->getFileNameURL($build),
'app' => $build->app,
'version' => $build->version,
'risk_id' => $build->risk_id,
'branch' => $build->branch,
'patch' => $build->patch,
'hash' => $build->hash,
'platform' => $build->platform,
'architecture' => $build->architecture,
'bitness' => $build->bitness,
'file_mtime' => $build->file_mtime,
'file_name' => $build->file_name,
'file_size' => $build->file_size,
'file_extension' => $build->file_extension,
'release_cycle' => $build->release_cycle,
);
}
echo(json_encode($builds_json, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
}
public function renderPage() { public function renderPage() {
$platform_filters = array( $platform_filters = array(
'windows', 'windows',
@ -257,7 +282,7 @@ class BuildsRenderer {
$request_uri = ($_SERVER['REQUEST_URI'] == "/" || $request_uri = ($_SERVER['REQUEST_URI'] == "/" ||
stringContains($_SERVER['REQUEST_URI'], "bpy")) ? stringContains($_SERVER['REQUEST_URI'], "bpy")) ?
"/download/daily" : $_SERVER['REQUEST_URI']; "/download/daily" : strtok($_SERVER['REQUEST_URI'], '?');
$env_id = getEnvId(); $env_id = getEnvId();

36
source/main.php
View File

@ -13,6 +13,29 @@ function handleNotFoundRequest() {
return; return;
} }
function renderDownloadResponseAsJSON($lister) {
header('Content-Type: application/json; charset=utf-8');
// Require v=1 to be specified
if (!isset($_GET['v']) || $_GET['v'] != '1') {
$data = [ 'error' => 'Missing supported format version, for example v=1'];
echo json_encode( $data );
http_response_code(400);
return;
}
$builds = $lister->getBuilds();
$renderer = new BuildsRenderer($builds);
$renderer->renderJSON();
return;
}
function renderDownloadResponseAsHTML($lister) {
// Serve the directory listing as HTML.
require 'templates/header.php';
Sergey Sharybin commented 2023-10-30 13:55:06 +01:00
Outdated
Review
Copy Link

You do not need to check isset($_GET['format']) when you're comparing to an explicit constant. Doing if ($_GET['format'] == 'json') { should suffice.

You do not need to check `isset($_GET['format'])` when you're comparing to an explicit constant. Doing `if ($_GET['format'] == 'json') {` should suffice.
Francesco Siddi commented 2023-10-31 21:48:02 +01:00
Outdated
Review
Copy Link

I tried, but removing the check raises:

Warning: Undefined array key "format" in /var/www/html/source/main.php on line 56

I tried, but removing the check raises: `Warning: Undefined array key "format" in /var/www/html/source/main.php on line 56`
// Logic inside content_build_lister.php will render the list of builds
require 'templates/content_build_lister.php';
fsiddi marked this conversation as resolved Outdated
Sergey Sharybin commented 2023-10-30 13:55:18 +01:00
Outdated
Review
Copy Link

Move to renderDownloadResponceAsJSON or something similar.

Move to `renderDownloadResponceAsJSON` or something similar.
require 'templates/footer.php';
}
function handleDownloadRequest() { function handleDownloadRequest() {
$lister = createListerForCurrentRequest(); $lister = createListerForCurrentRequest();
$is_valid = $lister->isValid(); $is_valid = $lister->isValid();
@ -29,11 +52,14 @@ function handleDownloadRequest() {
return; return;
} }
// Serve the directory listing. // Check if we are requesting a JSON formatted view.
require 'templates/header.php'; if (isset($_GET['format']) && $_GET['format'] == 'json') {
// Logic inside content_build_lister.php will render the list of builds renderDownloadResponseAsJSON($lister);
require 'templates/content_build_lister.php'; return;
require 'templates/footer.php'; }
renderDownloadResponseAsHTML($lister);
} }
function handleDashboardRequest() { function handleDashboardRequest() {