Skip to content

refactor: DRY cycle request context handling #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
somethingwithproof wants to merge 2 commits into from
Closed

refactor: DRY cycle request context handling #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2261ab4
refactor: DRY cycle request context handling
somethingwithproof Mar 15, 2026
31dfe04
fix: relax cycle request-context source assertion
somethingwithproof Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 21 additions & 33 deletions cycle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
include_once('./lib/utility.php');
include_once('./lib/api_data_source.php');
include_once('./plugins/cycle/functions.php');
include_once('./plugins/cycle/cycle_helpers.php');

set_default_action();

Expand Down Expand Up @@ -63,18 +64,16 @@ function cycle_graphs() {
global $id, $graph_id, $next_graph_id, $prev_graph_id;

$tree_list = get_allowed_trees();
$legend = get_request_var('legend');
$tree_id = get_request_var('tree_id');
$leaf_id = get_request_var('leaf_id');
$graphpp = get_request_var('graphs');
$cols = get_request_var('cols');
$rfilter = get_request_var('rfilter');
$id = get_request_var('id');
$width = get_request_var('width');
$height = get_request_var('height');

if (empty($tree_id)) $tree_id = db_fetch_cell('SELECT id FROM graph_tree ORDER BY name LIMIT 1');
if (empty($id)) $id = -1;
$request = cycle_get_request_context();
$legend = $request['legend'];
$tree_id = $request['tree_id'];
$leaf_id = $request['leaf_id'];
$graphpp = $request['graphs'];
$cols = $request['cols'];
$rfilter = $request['rfilter'];
$id = $request['id'];
$width = $request['width'];
$height = $request['height'];

/* get the start and end times for the graph */
$timespan = array();
Expand Down Expand Up @@ -154,26 +153,16 @@ function cycle() {
}

$tree_list = get_allowed_trees();
$legend = get_request_var('legend');
$tree_id = get_request_var('tree_id');
$leaf_id = get_request_var('leaf_id');
$graphpp = get_request_var('graphs');
$cols = get_request_var('cols');
$rfilter = get_request_var('rfilter');
$id = get_request_var('id');
$width = get_request_var('width');
$height = get_request_var('height');

if (empty($tree_id)) {
$tree_id = db_fetch_cell('SELECT id
FROM graph_tree
ORDER BY name
LIMIT 1');
}

if (empty($id)) {
$id = -1;
}
$request = cycle_get_request_context();
$legend = $request['legend'];
$tree_id = $request['tree_id'];
$leaf_id = $request['leaf_id'];
$graphpp = $request['graphs'];
$cols = $request['cols'];
$rfilter = $request['rfilter'];
$id = $request['id'];
$width = $request['width'];
$height = $request['height'];

/* get the start and end times for the graph */
$timespan = array();
Expand Down Expand Up @@ -401,4 +390,3 @@ function cycle() {

bottom_footer();
}

45 changes: 45 additions & 0 deletions cycle_helpers.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
| |
| This program is free software; you can redistribute it and/or |
| modify it under the terms of the GNU General Public License |
| as published by the Free Software Foundation; either version 2 |
| of the License, or (at your option) any later version. |
+-------------------------------------------------------------------------+
*/

if (!function_exists('cycle_get_default_tree_id')) {
function cycle_get_default_tree_id($tree_id) {
if (empty($tree_id)) {
$tree_id = db_fetch_cell('SELECT id
FROM graph_tree
ORDER BY name
LIMIT 1');
}

return $tree_id;
}
}

if (!function_exists('cycle_get_request_context')) {
function cycle_get_request_context() {
$context = array(
'legend' => get_request_var('legend'),
'tree_id' => get_request_var('tree_id'),
'leaf_id' => get_request_var('leaf_id'),
'graphs' => get_request_var('graphs'),
'cols' => get_request_var('cols'),
'rfilter' => get_request_var('rfilter'),
'id' => get_request_var('id'),
'width' => get_request_var('width'),
'height' => get_request_var('height')
);

$context['tree_id'] = cycle_get_default_tree_id($context['tree_id']);
$context['id'] = (empty($context['id']) ? -1 : $context['id']);

return $context;
}
}
86 changes: 86 additions & 0 deletions tests/test_request_context.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
| |
| Regression checks for shared cycle request-context helpers |
| |
| Run: php tests/test_request_context.php |
+-------------------------------------------------------------------------+
*/

$pass = 0;
$fail = 0;
$request_values = array();
$db_queries = array();

function assert_true($label, $value) {
global $pass, $fail;

if ($value) {
echo "PASS $label\n";
$pass++;
} else {
echo "FAIL $label\n";
$fail++;
}
}

function get_request_var($key) {
global $request_values;

return (array_key_exists($key, $request_values) ? $request_values[$key] : '');
}

function db_fetch_cell($sql) {
global $db_queries;
$db_queries[] = $sql;

return 777;
}

require_once __DIR__ . '/../cycle_helpers.php';

$request_values = array(
'legend' => 'true',
'tree_id' => '',
'leaf_id' => 19,
'graphs' => 8,
'cols' => 3,
'rfilter' => 'prod',
'id' => '',
'width' => 500,
'height' => 200
);
$db_queries = array();

$context = cycle_get_request_context();

assert_true('empty tree_id falls back to first tree query', $context['tree_id'] === 777);
assert_true('empty id normalizes to -1', $context['id'] === -1);
assert_true('legend is preserved', $context['legend'] === 'true');
assert_true('fallback query executes once', count($db_queries) === 1);

$request_values['tree_id'] = 123;
$request_values['id'] = 456;
$db_queries = array();

$context = cycle_get_request_context();

assert_true('provided tree_id is preserved', $context['tree_id'] === 123);
assert_true('provided id is preserved', $context['id'] === 456);
assert_true('no fallback query when tree_id exists', count($db_queries) === 0);

$cycle_source = file_get_contents(__DIR__ . '/../cycle.php');
assert_true('cycle.php is readable', $cycle_source !== false);
$cycle_source = ($cycle_source === false ? '' : $cycle_source);

assert_true(
'cycle.php uses shared request context helper',
preg_match('/cycle_get_request_context\\s*\\(/', $cycle_source) === 1
);

Comment on lines +74 to +82
Copy link
Copy Markdown
Author

@somethingwithproof somethingwithproof Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 31dfe04: replaced the count-based source assertion with a readability check plus a non-counted helper-usage assertion to avoid implementation-detail coupling.

echo "\n";
echo "Results: $pass passed, $fail failed\n";

exit($fail > 0 ? 1 : 0);