From bace2939af2b71c041b8e259e89d9f2826c0cefe Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:18:04 +1030 Subject: [PATCH 01/29] Show filter status and repair missing enabled state --- CHANGELOG.md | 2 + README.md | 4 ++ flowview.php | 3 +- flowview_filters.php | 8 ++++ flowview_schedules.php | 5 ++- flowview_upgrade.php | 1 + includes/database.php | 23 ++++++++++ includes/functions.php | 25 +++++++++++ locales/po/cacti.pot | 4 ++ setup.php | 1 + tests/Unit/FilterStatusTest.php | 78 +++++++++++++++++++++++++++++++++ tests/bootstrap-unit.php | 4 +- 12 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 tests/Unit/FilterStatusTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ca11bc..4020827 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ --- develop --- +* fix: Persist filter enabled state on fresh and existing installations, show enabled/disabled filter and schedule status using theme colours, and exclude disabled filters from report and schedule selection + * issue: Shorten the graph drilldown button title from 'View NetFlow Traffic In Range' to 'NetFlow' so it reads cleanly as a label in Cacti's graph action menu * dev: Create the plugin's config.php from config.php.dist for the flowview install step in the integration-test workflow (removed immediately afterwards so the unit tests still run with no config file), so the install no longer fails with "needs configuration" * feature: Add an optional FlowView Query Service (a long-running Go service under service/flowview-query) as a selectable Parallel Query Backend under FlowView Settings (default Legacy PHP Runner); when enabled, the parallel query parent runner (flowview_runner.php) delegates the whole query to the service over localhost instead of forking a PHP worker per shard, running the same cache-aware map-reduce (sharing parallel_database_query_shard_cache) and writing results back, with automatic fallback to the legacy runner when the service is unreachable diff --git a/README.md b/README.md index 8de558c..1acf529 100644 --- a/README.md +++ b/README.md @@ -211,3 +211,7 @@ resolution. ----------------------------------------------- Copyright (c) 2004-2026 - The Cacti Group, Inc. + +### Filter and schedule status + +Filters and schedules show Enabled in green and Disabled in grey, using the Cacti theme status classes. A disabled filter is excluded from report and schedule selectors and does not run scheduled reports. Disabling a filter does not stop its listener or delete history. Installations missing the filter state column retain existing filters as enabled when the column is created. diff --git a/flowview.php b/flowview.php index 0a2a903..8f7b369 100644 --- a/flowview.php +++ b/flowview.php @@ -34,6 +34,7 @@ require_once($config['base_path'] . '/lib/time.php'); flowview_connect(); +flowview_filter_status_upgrade(); set_default_action(); @@ -321,7 +322,7 @@ function load_session_for_filter() { WHERE id = ?', array(get_request_var('query'))); - if (cacti_sizeof($query)) { + if (cacti_sizeof($query) && ($query['enabled'] ?? '') === 'on') { foreach($query as $column => $value) { switch($column) { case 'name': diff --git a/flowview_filters.php b/flowview_filters.php index ca758a5..7fa71ec 100644 --- a/flowview_filters.php +++ b/flowview_filters.php @@ -34,6 +34,7 @@ require_once($config['base_path'] . '/lib/timespan_settings.php'); flowview_connect(); +flowview_filter_status_upgrade(); set_default_action(); @@ -382,6 +383,11 @@ function clearFilter() { 'align' => 'right', 'sort' => 'ASC' ), + 'nosort2' => array( + 'display' => __('Status', 'flowview'), + 'align' => 'center', + 'sort' => 'ASC' + ), 'nosort1' => array( 'display' => __('Resolution', 'flowview'), 'align' => 'right', @@ -421,6 +427,8 @@ function clearFilter() { form_selectable_cell($type, $filter['id']); form_selectable_cell($filter['id'], $filter['id'], '', 'right'); form_selectable_cell($sort, $filter['id'], '', 'right'); + $status = flowview_filter_status($filter); + form_selectable_cell('' . $status['label'] . '', $filter['id'], '', 'center'); form_selectable_cell($filter['resolve'], $filter['id'], '', 'right'); form_checkbox_cell($filter['name'], $filter['id']); diff --git a/flowview_schedules.php b/flowview_schedules.php index 77b3ea7..9517c18 100644 --- a/flowview_schedules.php +++ b/flowview_schedules.php @@ -32,6 +32,7 @@ require_once($config['base_path'] . '/plugins/flowview/includes/functions.php'); flowview_connect(); +flowview_filter_status_upgrade(); set_default_action(); @@ -59,6 +60,7 @@ $query_array = array_rekey( flowview_db_fetch_assoc('SELECT id, name FROM plugin_flowview_queries + WHERE enabled = "on" ORDER BY name'), 'id', 'name' ); @@ -1038,7 +1040,8 @@ function clearFilter() { form_selectable_cell('-', $row['id'], '', 'right'); } - form_selectable_cell(($row['enabled'] == 'on' ? "" . __('Yes', 'flowview') . "":"" . __('No', 'flowview') . ""), $row['id'], '', 'right'); + $status = flowview_filter_status($row); + form_selectable_cell('' . $status['label'] . '', $row['id'], '', 'right'); form_checkbox_cell($row['name'], $row['id']); form_end_row(); diff --git a/flowview_upgrade.php b/flowview_upgrade.php index a0e8224..6fc6994 100644 --- a/flowview_upgrade.php +++ b/flowview_upgrade.php @@ -30,6 +30,7 @@ require_once('./plugins/flowview/includes/database.php'); flowview_connect(); +flowview_filter_status_upgrade(); ini_set('max_execution_time', '0'); diff --git a/includes/database.php b/includes/database.php index b457a30..e6486c7 100644 --- a/includes/database.php +++ b/includes/database.php @@ -514,3 +514,26 @@ function flowview_db_get_table_column_types($table, mixed $cnn_id = false) { return $cols; } + +/** + * Repair the missing state column used by the existing filter bulk actions. + * Works for same-version upgrades and the configured FlowView connection. + * + * @return bool + */ +function flowview_filter_status_upgrade(): bool { + if (!flowview_db_table_exists('plugin_flowview_queries')) { + return false; + } + + if (flowview_db_column_exists('plugin_flowview_queries', 'enabled', false)) { + return true; + } + + if (flowview_db_execute("ALTER TABLE plugin_flowview_queries ADD COLUMN enabled char(2) NOT NULL DEFAULT 'on' AFTER name", false)) { + return true; + } + + // Another request may have completed the same upgrade concurrently. + return flowview_db_column_exists('plugin_flowview_queries', 'enabled', false); +} diff --git a/includes/functions.php b/includes/functions.php index bbe9f3f..a8ed126 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -730,6 +730,7 @@ function flowview_display_filter(): void { 'deviceDisabled', 'label' => __('Unavailable', 'flowview')]; + } + + return $record['enabled'] === 'on' + ? ['class' => 'deviceUp', 'label' => __('Enabled', 'flowview')] + : ['class' => 'deviceDisabled', 'label' => __('Disabled', 'flowview')]; +} diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index 23efd5d..de03c52 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -2794,3 +2794,7 @@ msgstr "" #: setup.php msgid "NetFlow" msgstr "" + +#: includes/functions.php +msgid "Unavailable" +msgstr "" diff --git a/setup.php b/setup.php index 8726c0f..283616b 100644 --- a/setup.php +++ b/setup.php @@ -1147,6 +1147,7 @@ function flowview_setup_table(): void { flowview_db_execute("CREATE TABLE IF NOT EXISTS `plugin_flowview_queries` ( id int(11) unsigned NOT NULL AUTO_INCREMENT, name varchar(255) NOT NULL, + enabled char(2) NOT NULL DEFAULT 'on', device_id int(11) unsigned NOT NULL, template_id int(11) NOT NULL DEFAULT '0', ex_addr varchar(46) NOT NULL DEFAULT '', diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php new file mode 100644 index 0000000..1e0bc92 --- /dev/null +++ b/tests/Unit/FilterStatusTest.php @@ -0,0 +1,78 @@ + 'on']))->toBe(['class' => 'deviceUp', 'label' => 'Enabled']) + ->and(flowview_filter_status(['enabled' => '']))->toBe(['class' => 'deviceDisabled', 'label' => 'Disabled']) + ->and(flowview_filter_status([]))->toBe(['class' => 'deviceDisabled', 'label' => 'Unavailable']); +}); + +it('does not alter a missing filter table', function () { + expect(flowview_filter_status_upgrade())->toBeFalse(); + expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('db_execute'); +}); + +it('leaves an existing enabled column intact', function () { + plugin_test_queue_db_result('db_fetch_cell', 1); + plugin_test_queue_db_result('db_column_exists', true); + expect(flowview_filter_status_upgrade())->toBeTrue(); + expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('db_execute'); +}); + +it('repairs legacy filters as enabled through the configured database', function () { + plugin_test_queue_db_result('db_fetch_cell', 1); + expect(flowview_filter_status_upgrade())->toBeTrue(); + $writes = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($call) => $call['fn'] === 'db_execute')); + expect($writes)->toHaveCount(1) + ->and($writes[0]['sql'])->toBe("ALTER TABLE plugin_flowview_queries ADD COLUMN enabled char(2) NOT NULL DEFAULT 'on' AFTER name") + ->and($writes[0]['conn'])->toBe('filter-database'); +}); + +it('rechecks concurrent upgrades and reports actual failures', function ($exists) { + plugin_test_queue_db_result('db_fetch_cell', 1); + plugin_test_queue_db_result('db_column_exists', false); + plugin_test_queue_db_result('db_column_exists', $exists); + plugin_test_queue_db_result('db_execute', false); + expect(flowview_filter_status_upgrade())->toBe($exists); +})->with([true, false]); + +it('does not run a schedule when the state schema is unavailable', function () { + plugin_flowview_run_schedule(1, 1); + expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('db_fetch_row_prepared'); +}); + +it('does not prepare scheduled reports for missing or disabled filters', function ($query) { + plugin_test_queue_db_result('db_fetch_cell', 1); + plugin_test_queue_db_result('db_column_exists', true); + plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 7]); + plugin_test_queue_db_result('db_fetch_row_prepared', $query); + plugin_flowview_run_schedule(1, 1); + $reads = array_filter($GLOBALS['__test_db_calls'], fn ($call) => $call['fn'] === 'db_fetch_row_prepared'); + expect($reads)->toHaveCount(2); +})->with([[[]], [['enabled' => '']], [['name' => 'legacy']]]); + +// Stop at time selection so the positive path cannot send a report in unit tests. +if (!function_exists('read_user_setting')) { + function read_user_setting($name) { return false; } +} +if (!function_exists('get_timespan')) { + function get_timespan(&$span, $now, $timespan, $weekday) { + throw new RuntimeException('enabled report reached time selection'); + } +} + +it('continues preparing an enabled scheduled filter', function () { + plugin_test_queue_db_result('db_fetch_cell', 1); + plugin_test_queue_db_result('db_column_exists', true); + plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 7]); + plugin_test_queue_db_result('db_fetch_row_prepared', ['enabled' => 'on', 'timespan' => 1]); + expect(fn () => plugin_flowview_run_schedule(1, 1))->toThrow(RuntimeException::class, 'enabled report reached time selection'); +}); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index c83ff97..ee66038 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -123,7 +123,7 @@ function plugin_test_db_result($function, $default) { if (!function_exists('db_execute')) { function db_execute($sql, $log = true, $conn = false) { $GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute', 'sql' => $sql, 'params' => array(), 'conn' => $conn); - return true; + return plugin_test_db_result('db_execute', true); } } @@ -201,7 +201,7 @@ function db_index_exists($table, $index) { if (!function_exists('db_column_exists')) { function db_column_exists($table, $column) { - return false; + return plugin_test_db_result('db_column_exists', false); } } From 56ac1350fd5e76440fceaf53782d71723a887c0e Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:21:16 +1030 Subject: [PATCH 02/29] Remove unused sorting direction from status header --- flowview_filters.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/flowview_filters.php b/flowview_filters.php index 7fa71ec..b6643bd 100644 --- a/flowview_filters.php +++ b/flowview_filters.php @@ -385,8 +385,7 @@ function clearFilter() { ), 'nosort2' => array( 'display' => __('Status', 'flowview'), - 'align' => 'center', - 'sort' => 'ASC' + 'align' => 'center' ), 'nosort1' => array( 'display' => __('Resolution', 'flowview'), From ce14e35c91a50a132da852704444b4a6e6d5fb4c Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:55:52 +1030 Subject: [PATCH 03/29] Repair filter schema in background and preserve schedule associations --- flowview.php | 7 ++++++- flowview_filters.php | 7 ++++++- flowview_schedules.php | 15 +++++++-------- flowview_upgrade.php | 18 +++++++++++------- includes/database.php | 20 ++++++++------------ includes/functions.php | 14 +++++++++++++- locales/po/cacti.pot | 8 ++++++++ setup.php | 4 ++-- tests/Unit/FilterStatusTest.php | 25 +++++++++++++++++++------ 9 files changed, 80 insertions(+), 38 deletions(-) diff --git a/flowview.php b/flowview.php index 8f7b369..b34dc68 100644 --- a/flowview.php +++ b/flowview.php @@ -34,7 +34,12 @@ require_once($config['base_path'] . '/lib/time.php'); flowview_connect(); -flowview_filter_status_upgrade(); +if (!flowview_filter_status_ready()) { + plugin_flowview_check_upgrade(); + http_response_code(503); + print html_escape(__('FlowView filter schema upgrade is pending. Please try again shortly.', 'flowview')); + exit; +} set_default_action(); diff --git a/flowview_filters.php b/flowview_filters.php index b6643bd..5e016c9 100644 --- a/flowview_filters.php +++ b/flowview_filters.php @@ -34,7 +34,12 @@ require_once($config['base_path'] . '/lib/timespan_settings.php'); flowview_connect(); -flowview_filter_status_upgrade(); +if (!flowview_filter_status_ready()) { + plugin_flowview_check_upgrade(); + http_response_code(503); + print html_escape(__('FlowView filter schema upgrade is pending. Please try again shortly.', 'flowview')); + exit; +} set_default_action(); diff --git a/flowview_schedules.php b/flowview_schedules.php index 9517c18..fd3853f 100644 --- a/flowview_schedules.php +++ b/flowview_schedules.php @@ -32,7 +32,12 @@ require_once($config['base_path'] . '/plugins/flowview/includes/functions.php'); flowview_connect(); -flowview_filter_status_upgrade(); +if (!flowview_filter_status_ready()) { + plugin_flowview_check_upgrade(); + http_response_code(503); + print html_escape(__('FlowView filter schema upgrade is pending. Please try again shortly.', 'flowview')); + exit; +} set_default_action(); @@ -57,13 +62,7 @@ $formats = reports_get_format_files(); -$query_array = array_rekey( - flowview_db_fetch_assoc('SELECT id, name - FROM plugin_flowview_queries - WHERE enabled = "on" - ORDER BY name'), - 'id', 'name' -); +$query_array = flowview_schedule_filters((int) get_filter_request_var('id')); if (db_table_exists('plugin_notification_lists')) { $notification_lists = array_rekey( diff --git a/flowview_upgrade.php b/flowview_upgrade.php index 6fc6994..f7bf10e 100644 --- a/flowview_upgrade.php +++ b/flowview_upgrade.php @@ -30,7 +30,6 @@ require_once('./plugins/flowview/includes/database.php'); flowview_connect(); -flowview_filter_status_upgrade(); ini_set('max_execution_time', '0'); @@ -82,12 +81,6 @@ $info = plugin_flowview_version(); $current_version = $info['version']; -/* do a version check */ -if ($forcever == '' && $old_version == $current_version) { - cacti_log('Your Flowview is already up to date (v' . $current_version . ' vs v' . $old_version . ') not upgrading. Use --forcever to override', true, 'FLOWVIEW'); - exit(0); -} - if (!register_process_start('flowview', 'upgrade', 1, 86400)) { if (empty($forcever)) { print 'WARNING: Running process detected. To override use --forcever' . PHP_EOL; @@ -95,6 +88,17 @@ } } +if (!flowview_filter_status_upgrade()) { + cacti_log('FlowView filter state upgrade failed; it will be retried.', true, 'FLOWVIEW'); + exit(1); +} + +/* do a version check */ +if ($forcever == '' && $old_version == $current_version) { + cacti_log('Your Flowview is already up to date (v' . $current_version . ' vs v' . $old_version . ') not upgrading. Use --forcever to override', true, 'FLOWVIEW'); + exit(0); +} + cacti_log('Upgrading from v' . $old_version . ' to ' . $current_version, true, 'FLOWVIEW'); flowview_upgrade($current_version, $old_version); diff --git a/includes/database.php b/includes/database.php index e6486c7..5dfe0f4 100644 --- a/includes/database.php +++ b/includes/database.php @@ -515,25 +515,21 @@ function flowview_db_get_table_column_types($table, mixed $cnn_id = false) { return $cols; } -/** - * Repair the missing state column used by the existing filter bulk actions. - * Works for same-version upgrades and the configured FlowView connection. - * - * @return bool - */ +/** Check live metadata without Cacti's per-request column cache. */ +function flowview_filter_status_ready(): bool { + return (bool) flowview_db_fetch_row("SHOW COLUMNS FROM plugin_flowview_queries LIKE 'enabled'", false); +} + +/** Repair legacy state only from the background upgrade process. */ function flowview_filter_status_upgrade(): bool { if (!flowview_db_table_exists('plugin_flowview_queries')) { return false; } - - if (flowview_db_column_exists('plugin_flowview_queries', 'enabled', false)) { + if (flowview_filter_status_ready()) { return true; } - if (flowview_db_execute("ALTER TABLE plugin_flowview_queries ADD COLUMN enabled char(2) NOT NULL DEFAULT 'on' AFTER name", false)) { return true; } - - // Another request may have completed the same upgrade concurrently. - return flowview_db_column_exists('plugin_flowview_queries', 'enabled', false); + return flowview_filter_status_ready(); } diff --git a/includes/functions.php b/includes/functions.php index a8ed126..99cc51f 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -1921,7 +1921,7 @@ function get_port_name($port_num, int $port_proto = 6) { function plugin_flowview_run_schedule($id, $report_id): void { global $config; - if (!flowview_filter_status_upgrade()) { + if (!flowview_filter_status_ready()) { return; } @@ -7362,3 +7362,15 @@ function flowview_filter_status(array $record): array { ? ['class' => 'deviceUp', 'label' => __('Enabled', 'flowview')] : ['class' => 'deviceDisabled', 'label' => __('Disabled', 'flowview')]; } + +/** Enabled choices plus the existing association when editing a schedule. */ +function flowview_schedule_filters(int $schedule_id): array { + $rows = flowview_db_fetch_assoc_prepared('SELECT id, name, enabled FROM plugin_flowview_queries + WHERE enabled = "on" OR id = (SELECT query_id FROM plugin_flowview_schedules WHERE id = ?) + ORDER BY name', [$schedule_id]); + $options = []; + foreach ($rows ?: [] as $row) { + $options[$row['id']] = $row['enabled'] === 'on' ? $row['name'] : sprintf(__('%s (Disabled)', 'flowview'), $row['name']); + } + return $options; +} diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index de03c52..730c7b5 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -2798,3 +2798,11 @@ msgstr "" #: includes/functions.php msgid "Unavailable" msgstr "" + +#: includes/functions.php +msgid "FlowView filter schema upgrade is pending. Please try again shortly." +msgstr "" + +#: includes/functions.php +msgid "%s (Disabled)" +msgstr "" diff --git a/setup.php b/setup.php index 283616b..7698b15 100644 --- a/setup.php +++ b/setup.php @@ -130,7 +130,7 @@ function plugin_flowview_upgrade(): bool { function plugin_flowview_check_upgrade(bool $force = false): void { global $config; - $files = ['plugins.php', 'flowview.php', 'index.php']; + $files = ['plugins.php', 'flowview.php', 'flowview_filters.php', 'flowview_schedules.php', 'index.php']; if (isset($_SERVER['PHP_SELF']) && !in_array(basename($_SERVER['PHP_SELF']), $files)) { return; } @@ -146,7 +146,7 @@ function plugin_flowview_check_upgrade(bool $force = false): void { FROM plugin_config WHERE directory = ?', array('flowview')); - if ($current != $old || $force) { + if ($current != $old || $force || !flowview_filter_status_ready()) { $php_binary = read_config_option('path_php_binary'); exec_background($php_binary, '-q ' .$config['base_path'] . '/plugins/flowview/flowview_upgrade.php'); diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index 1e0bc92..01aa960 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -22,7 +22,7 @@ it('leaves an existing enabled column intact', function () { plugin_test_queue_db_result('db_fetch_cell', 1); - plugin_test_queue_db_result('db_column_exists', true); + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); expect(flowview_filter_status_upgrade())->toBeTrue(); expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('db_execute'); }); @@ -38,8 +38,8 @@ it('rechecks concurrent upgrades and reports actual failures', function ($exists) { plugin_test_queue_db_result('db_fetch_cell', 1); - plugin_test_queue_db_result('db_column_exists', false); - plugin_test_queue_db_result('db_column_exists', $exists); + plugin_test_queue_db_result('db_fetch_row', []); + plugin_test_queue_db_result('db_fetch_row', $exists ? ['Field' => 'enabled'] : []); plugin_test_queue_db_result('db_execute', false); expect(flowview_filter_status_upgrade())->toBe($exists); })->with([true, false]); @@ -51,7 +51,7 @@ it('does not prepare scheduled reports for missing or disabled filters', function ($query) { plugin_test_queue_db_result('db_fetch_cell', 1); - plugin_test_queue_db_result('db_column_exists', true); + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 7]); plugin_test_queue_db_result('db_fetch_row_prepared', $query); plugin_flowview_run_schedule(1, 1); @@ -61,7 +61,9 @@ // Stop at time selection so the positive path cannot send a report in unit tests. if (!function_exists('read_user_setting')) { - function read_user_setting($name) { return false; } + function read_user_setting($name) { + return false; + } } if (!function_exists('get_timespan')) { function get_timespan(&$span, $now, $timespan, $weekday) { @@ -71,8 +73,19 @@ function get_timespan(&$span, $now, $timespan, $weekday) { it('continues preparing an enabled scheduled filter', function () { plugin_test_queue_db_result('db_fetch_cell', 1); - plugin_test_queue_db_result('db_column_exists', true); + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 7]); plugin_test_queue_db_result('db_fetch_row_prepared', ['enabled' => 'on', 'timespan' => 1]); expect(fn () => plugin_flowview_run_schedule(1, 1))->toThrow(RuntimeException::class, 'enabled report reached time selection'); }); + +it('preserves an existing disabled schedule filter with an explicit label', function () { + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 2, 'name' => 'Active', 'enabled' => 'on'], ['id' => 7, 'name' => 'Saved', 'enabled' => '']]); + expect(flowview_schedule_filters(3))->toBe([2 => 'Active', 7 => 'Saved (Disabled)']); + $call = $GLOBALS['__test_db_calls'][0]; + expect($call['params'])->toBe([3])->and($call['sql'])->toContain('SELECT query_id'); +}); +it('handles unavailable schedule choices without replacing the association', function () { + plugin_test_queue_db_result('db_fetch_assoc_prepared', false); + expect(flowview_schedule_filters(3))->toBe([]); +}); From e02dfb4f27977f0b14014a0bfd4cbb4fb3889720 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:57:58 +1030 Subject: [PATCH 04/29] Share pending filter upgrade handling across web entry points --- README.md | 2 ++ flowview.php | 7 +------ flowview_filters.php | 7 +------ flowview_schedules.php | 7 +------ setup.php | 11 +++++++++++ 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 1acf529..8eaf4dc 100644 --- a/README.md +++ b/README.md @@ -215,3 +215,5 @@ Copyright (c) 2004-2026 - The Cacti Group, Inc. ### Filter and schedule status Filters and schedules show Enabled in green and Disabled in grey, using the Cacti theme status classes. A disabled filter is excluded from report and schedule selectors and does not run scheduled reports. Disabling a filter does not stop its listener or delete history. Installations missing the filter state column retain existing filters as enabled when the column is created. + +Missing filter-state columns are repaired by the background upgrade process, including same-version repairs. Pages return a retry message until repair completes; scheduled reports defer without issuing schema changes. Editing a schedule retains its existing disabled filter and labels it Disabled. diff --git a/flowview.php b/flowview.php index b34dc68..b86a345 100644 --- a/flowview.php +++ b/flowview.php @@ -34,12 +34,7 @@ require_once($config['base_path'] . '/lib/time.php'); flowview_connect(); -if (!flowview_filter_status_ready()) { - plugin_flowview_check_upgrade(); - http_response_code(503); - print html_escape(__('FlowView filter schema upgrade is pending. Please try again shortly.', 'flowview')); - exit; -} +flowview_require_filter_status(); set_default_action(); diff --git a/flowview_filters.php b/flowview_filters.php index 5e016c9..8559397 100644 --- a/flowview_filters.php +++ b/flowview_filters.php @@ -34,12 +34,7 @@ require_once($config['base_path'] . '/lib/timespan_settings.php'); flowview_connect(); -if (!flowview_filter_status_ready()) { - plugin_flowview_check_upgrade(); - http_response_code(503); - print html_escape(__('FlowView filter schema upgrade is pending. Please try again shortly.', 'flowview')); - exit; -} +flowview_require_filter_status(); set_default_action(); diff --git a/flowview_schedules.php b/flowview_schedules.php index fd3853f..3501b0e 100644 --- a/flowview_schedules.php +++ b/flowview_schedules.php @@ -32,12 +32,7 @@ require_once($config['base_path'] . '/plugins/flowview/includes/functions.php'); flowview_connect(); -if (!flowview_filter_status_ready()) { - plugin_flowview_check_upgrade(); - http_response_code(503); - print html_escape(__('FlowView filter schema upgrade is pending. Please try again shortly.', 'flowview')); - exit; -} +flowview_require_filter_status(); set_default_action(); diff --git a/setup.php b/setup.php index 7698b15..39d7acb 100644 --- a/setup.php +++ b/setup.php @@ -158,6 +158,17 @@ function plugin_flowview_check_upgrade(bool $force = false): void { } } +/** Keep ordinary page requests out of metadata-lock waits during repair. */ +function flowview_require_filter_status(): void { + if (flowview_filter_status_ready()) { + return; + } + plugin_flowview_check_upgrade(); + http_response_code(503); + print html_escape(__('FlowView filter schema upgrade is pending. Please try again shortly.', 'flowview')); + exit; +} + /** * @return mixed */ From 333d835be903b0aca096176ebe8305a09f76669c Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:08:37 +1030 Subject: [PATCH 05/29] Format disabled filter labels consistently with Cacti gettext --- includes/functions.php | 2 +- locales/po/cacti.pot | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/includes/functions.php b/includes/functions.php index 99cc51f..9ad4602 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -7370,7 +7370,7 @@ function flowview_schedule_filters(int $schedule_id): array { ORDER BY name', [$schedule_id]); $options = []; foreach ($rows ?: [] as $row) { - $options[$row['id']] = $row['enabled'] === 'on' ? $row['name'] : sprintf(__('%s (Disabled)', 'flowview'), $row['name']); + $options[$row['id']] = $row['enabled'] === 'on' ? $row['name'] : $row['name'] . ' (' . __('Disabled', 'flowview') . ')'; } return $options; } diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index 730c7b5..796c69f 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -2802,7 +2802,3 @@ msgstr "" #: includes/functions.php msgid "FlowView filter schema upgrade is pending. Please try again shortly." msgstr "" - -#: includes/functions.php -msgid "%s (Disabled)" -msgstr "" From df7549a7283e92bceb30a65ee2293b6a5a549c66 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:43:34 +1030 Subject: [PATCH 06/29] Preserve scheduled reports while filter migration is pending --- README.md | 2 + flowview_upgrade.php | 2 +- includes/functions-pre13.php | 6 ++- includes/functions.php | 19 ++++++++++ phpunit.xml | 1 + run_schedule.php | 17 ++++++++- tests/Unit/FilterScheduleRetryTest.php | 52 ++++++++++++++++++++++++++ tests/Unit/FilterStatusTest.php | 21 +++++------ tests/bootstrap-unit.php | 27 ++++++++++++- 9 files changed, 130 insertions(+), 17 deletions(-) create mode 100644 tests/Unit/FilterScheduleRetryTest.php diff --git a/README.md b/README.md index 8eaf4dc..dcfb3cf 100644 --- a/README.md +++ b/README.md @@ -217,3 +217,5 @@ Copyright (c) 2004-2026 - The Cacti Group, Inc. Filters and schedules show Enabled in green and Disabled in grey, using the Cacti theme status classes. A disabled filter is excluded from report and schedule selectors and does not run scheduled reports. Disabling a filter does not stop its listener or delete history. Installations missing the filter state column retain existing filters as enabled when the column is created. Missing filter-state columns are repaired by the background upgrade process, including same-version repairs. Pages return a retry message until repair completes; scheduled reports defer without issuing schema changes. Editing a schedule retains its existing disabled filter and labels it Disabled. + +Pending filter-state migration leaves scheduled queue entries and lastsent/start timestamps unchanged. The pre-1.3 queue shim checks readiness before claiming work; a directly dispatched child preserves a replacement pending entry before its claimed row can be removed by a core runner. diff --git a/flowview_upgrade.php b/flowview_upgrade.php index f7bf10e..3cbf242 100644 --- a/flowview_upgrade.php +++ b/flowview_upgrade.php @@ -23,7 +23,7 @@ +-------------------------------------------------------------------------+ */ -chdir('../../'); +chdir(__DIR__ . '/../../'); require('./include/cli_check.php'); require_once('./plugins/flowview/includes/functions.php'); require_once('./plugins/flowview/setup.php'); diff --git a/includes/functions-pre13.php b/includes/functions-pre13.php index 1a5f1c5..2ee2264 100644 --- a/includes/functions-pre13.php +++ b/includes/functions-pre13.php @@ -262,14 +262,16 @@ function reports_queue($name, $request_type, $source, $source_id, $command, $not function reports_run($id): bool { global $config; - require_once($config['base_path'] . '/lib/poller.php'); - $report = db_fetch_row_prepared('SELECT * FROM reports_queued WHERE id = ?', [$id]); if (cacti_sizeof($report)) { + if ($report['source'] === 'flowview' && !flowview_filter_status_ready()) { + return false; + } + require_once($config['base_path'] . '/lib/poller.php'); db_execute_prepared('UPDATE reports_queued SET status = ?, start_time = ? WHERE id = ?', diff --git a/includes/functions.php b/includes/functions.php index 9ad4602..b2006a0 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -7374,3 +7374,22 @@ function flowview_schedule_filters(int $schedule_id): array { } return $options; } + +/** Preserve a pending occurrence if a queue runner has already claimed its row. */ +function flowview_retry_queued_report(array $report): bool { + if (($report['source'] ?? '') !== 'flowview' || empty($report['id'])) { + return false; + } + if (($report['status'] ?? '') === 'pending') { + return true; + } + $retry = $report; + $retry['id'] = 0; + $retry['status'] = 'pending'; + unset($retry['start_time']); + if ((int) sql_save($retry, 'reports_queued') <= 0) { + cacti_log('Unable to preserve the queued FlowView occurrence for retry.', false, 'FLOWVIEW'); + return false; + } + return true; +} diff --git a/phpunit.xml b/phpunit.xml index c9c7693..fa89edd 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -28,6 +28,7 @@ includes/functions.php + includes/functions-pre13.php includes/database.php diff --git a/run_schedule.php b/run_schedule.php index 17c052c..cafbdc3 100644 --- a/run_schedule.php +++ b/run_schedule.php @@ -86,6 +86,12 @@ } } +// Do not claim pending rows while the filter-state schema is unavailable. +if ($scheduled && !flowview_filter_status_ready()) { + cacti_log('FlowView scheduled reports deferred until the filter-state upgrade completes.', false, 'FLOWVIEW'); + exit(75); +} + if ($scheduled == true) { /* silently end if the registered process is still running, or process table missing */ if (!$force) { @@ -125,7 +131,9 @@ ['flowview', 'pending']); if (cacti_sizeof($report)) { - reports_run($report['id']); + if (!flowview_filter_status_ready() || reports_run($report['id']) === false) { + break; + } $run++; } else { @@ -158,6 +166,13 @@ if (cacti_sizeof($report)) { $id = $report['source_id']; + if (!flowview_filter_status_ready()) { + // Core queue runners may delete the claimed row after this child exits. + // Preserve a new pending row first, without touching schedule timestamps. + flowview_retry_queued_report($report); + exit(75); + } + /* silently end if the registered process is still running, or process table missing */ if (!$force) { if (!register_process_start('flowsched', "child_$id", 0, 1200)) { diff --git a/tests/Unit/FilterScheduleRetryTest.php b/tests/Unit/FilterScheduleRetryTest.php new file mode 100644 index 0000000..edbfcb2 --- /dev/null +++ b/tests/Unit/FilterScheduleRetryTest.php @@ -0,0 +1,52 @@ + 51, 'source' => 'flowview']); + expect(reports_run(51))->toBeFalse(); + expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('db_execute_prepared', 'exec_with_timeout'); + expect($GLOBALS['__test_saved_rows'])->toBe([]); +}); +it('preserves an already claimed occurrence under a new pending queue id', function () { + $original = ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'status' => 'running', 'start_time' => '2026-10-18 00:00:00', 'scheduled_time' => '2026-10-17 23:00:00', 'run_command' => 'php run_schedule.php', 'notification' => '{}']; + plugin_test_queue_db_result('sql_save', 72); + expect(flowview_retry_queued_report($original))->toBeTrue(); + $saved = $GLOBALS['__test_saved_rows'][0]; + expect($saved['table'])->toBe('reports_queued')->and($saved['row']['id'])->toBe(0)->and($saved['row']['status'])->toBe('pending'); + expect($saved['row'])->not->toHaveKey('start_time'); + foreach (['source_id', 'scheduled_time', 'run_command', 'notification'] as $key) { + expect($saved['row'][$key])->toBe($original[$key]); + } + expect($GLOBALS['__test_db_calls'])->toBe([]); +}); +it('does not duplicate a row which is already pending', function () { + expect(flowview_retry_queued_report(['id' => 51, 'source' => 'flowview', 'status' => 'pending']))->toBeTrue(); + expect($GLOBALS['__test_saved_rows'])->toBe([]); +}); +it('rejects unrelated or unidentified queued rows', function ($row) { + expect(flowview_retry_queued_report($row))->toBeFalse(); + expect($GLOBALS['__test_saved_rows'])->toBe([]); +})->with([[[]], [['id' => 51, 'source' => 'other']]]); +it('reports a failed preservation instead of claiming a successful retry', function () { + plugin_test_queue_db_result('sql_save', false); + expect(flowview_retry_queued_report(['id' => 51, 'source' => 'flowview', 'status' => 'running']))->toBeFalse(); +}); +it('continues normal pre-1.3 queue processing once the schema is ready', function () { + global $config; + $old = $config['base_path']; + $root = sys_get_temp_dir() . '/flowview-shim-' . bin2hex(random_bytes(8)); + mkdir($root . '/lib', 0700, true); + file_put_contents($root . '/lib/poller.php', ' 51, 'source' => 'flowview', 'name' => 'Fixture', 'source_id' => 7, 'run_command' => 'fixture-command', 'run_timeout' => 60]); + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + expect(reports_run(51))->toBeTrue(); + $sql = implode(' ', array_column($GLOBALS['__test_db_calls'], 'sql')); + expect($sql)->toContain('UPDATE reports_queued', 'DELETE FROM reports_queued'); + } finally { + $config['base_path'] = $old; + unlink($root . '/lib/poller.php'); + rmdir($root . '/lib'); + rmdir($root); + } +}); diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index 01aa960..6dbb666 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -59,23 +59,12 @@ expect($reads)->toHaveCount(2); })->with([[[]], [['enabled' => '']], [['name' => 'legacy']]]); -// Stop at time selection so the positive path cannot send a report in unit tests. -if (!function_exists('read_user_setting')) { - function read_user_setting($name) { - return false; - } -} -if (!function_exists('get_timespan')) { - function get_timespan(&$span, $now, $timespan, $weekday) { - throw new RuntimeException('enabled report reached time selection'); - } -} - it('continues preparing an enabled scheduled filter', function () { plugin_test_queue_db_result('db_fetch_cell', 1); plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 7]); plugin_test_queue_db_result('db_fetch_row_prepared', ['enabled' => 'on', 'timespan' => 1]); + plugin_test_queue_db_result('get_timespan', new RuntimeException('enabled report reached time selection')); expect(fn () => plugin_flowview_run_schedule(1, 1))->toThrow(RuntimeException::class, 'enabled report reached time selection'); }); @@ -89,3 +78,11 @@ function get_timespan(&$span, $now, $timespan, $weekday) { plugin_test_queue_db_result('db_fetch_assoc_prepared', false); expect(flowview_schedule_filters(3))->toBe([]); }); + +it('resets a queued time-selection exception between tests', function () { + plugin_test_queue_db_result('get_timespan', new RuntimeException('fixture only')); + plugin_test_reset(); + $span = []; + get_timespan($span, 2000, 1, 0); + expect($span)->toBe(['begin_now' => 200, 'end_now' => 2000]); +}); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index ee66038..331e29f 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -95,6 +95,8 @@ function plugin_test_reset() { $GLOBALS['__test_db_results'] = array(); $GLOBALS['__test_config'] = array(); $GLOBALS['__test_messages'] = array(); + $GLOBALS['__test_saved_rows'] = []; + $GLOBALS['__test_user_settings'] = []; $_GET = array(); $_POST = array(); $_REQUEST = array(); @@ -394,7 +396,8 @@ function is_error_message() { if (!function_exists('sql_save')) { function sql_save($array, $table, $key = 'id') { - return isset($array['id']) ? $array['id'] : 1; + $GLOBALS['__test_saved_rows'][] = ['table' => $table, 'row' => $array]; + return plugin_test_db_result('sql_save', isset($array['id']) ? $array['id'] : 1); } } @@ -471,3 +474,25 @@ function flowview_test_load($path) { require_once dirname(__DIR__) . '/includes/functions.php'; require_once dirname(__DIR__) . '/includes/database.php'; + +// Core helper doubles are reset through plugin_test_reset(), never test collection. +if (!function_exists('read_user_setting')) { + function read_user_setting($name, $default = false) { + return $GLOBALS['__test_user_settings'][$name] ?? $default; + } +} +if (!function_exists('get_timespan')) { + function get_timespan(&$span, $now, $timespan, $weekday) { + $result = plugin_test_db_result('get_timespan', ['begin_now' => $now - 1800, 'end_now' => $now]); + if ($result instanceof Throwable) { + throw $result; + } + $span = $result; + } +} +if (!function_exists('exec_with_timeout')) { + function exec_with_timeout($command, &$output, &$status, $timeout = 5) { + $GLOBALS['__test_db_calls'][] = ['fn' => 'exec_with_timeout', 'sql' => '', 'params' => [$command, $timeout]]; + $status = plugin_test_db_result('exec_with_timeout', 0); + } +} From 3d38365f0498059f0fe2a7bd3de5895d3e6a0525 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:47:21 +1030 Subject: [PATCH 07/29] Preserve the core process helper return contract in tests --- tests/bootstrap-unit.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 331e29f..3854542 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -494,5 +494,6 @@ function get_timespan(&$span, $now, $timespan, $weekday) { function exec_with_timeout($command, &$output, &$status, $timeout = 5) { $GLOBALS['__test_db_calls'][] = ['fn' => 'exec_with_timeout', 'sql' => '', 'params' => [$command, $timeout]]; $status = plugin_test_db_result('exec_with_timeout', 0); + return plugin_test_db_result('exec_with_timeout_result', null); } } From d829bd3bdd3df4ddb52e4d747fb5236a554cb7a6 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 18 Oct 2026 22:27:56 +1030 Subject: [PATCH 08/29] Retain original queue occurrences through FlowView-owned dispatch --- README.md | 2 +- includes/functions-pre13.php | 4 + includes/functions.php | 46 ++++++-- run_schedule.php | 12 +- tests/Unit/FilterScheduleRetryTest.php | 146 +++++++++++++++++++------ tests/bootstrap-unit.php | 2 +- 6 files changed, 158 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index dcfb3cf..1035367 100644 --- a/README.md +++ b/README.md @@ -218,4 +218,4 @@ Filters and schedules show Enabled in green and Disabled in grey, using the Cact Missing filter-state columns are repaired by the background upgrade process, including same-version repairs. Pages return a retry message until repair completes; scheduled reports defer without issuing schema changes. Editing a schedule retains its existing disabled filter and labels it Disabled. -Pending filter-state migration leaves scheduled queue entries and lastsent/start timestamps unchanged. The pre-1.3 queue shim checks readiness before claiming work; a directly dispatched child preserves a replacement pending entry before its claimed row can be removed by a core runner. +Pending filter-state migration leaves scheduled queue entries and lastsent/start timestamps unchanged. FlowView uses its own dispatcher on both Cacti versions: deferred or failed children retain their original occurrence, without saving a replacement. Native queue IDs and the older report-ID convention are both accepted. If resetting the retained row to pending fails, the row is kept and a queue-repair error is logged. diff --git a/includes/functions-pre13.php b/includes/functions-pre13.php index 2ee2264..48562c7 100644 --- a/includes/functions-pre13.php +++ b/includes/functions-pre13.php @@ -297,6 +297,10 @@ function reports_run($id): bool { cacti_log($stats, false, 'SYSTEM'); + if ($report['source'] === 'flowview' && ($last_line === false || $return_code !== 0)) { + db_execute_prepared('UPDATE reports_queued SET status = ? WHERE id = ?', ['pending', $id]); + return false; + } db_execute_prepared('DELETE FROM reports_queued WHERE id = ?', [$id]); return true; } diff --git a/includes/functions.php b/includes/functions.php index b2006a0..72f2852 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -7375,21 +7375,43 @@ function flowview_schedule_filters(int $schedule_id): array { return $options; } -/** Preserve a pending occurrence if a queue runner has already claimed its row. */ -function flowview_retry_queued_report(array $report): bool { - if (($report['source'] ?? '') !== 'flowview' || empty($report['id'])) { +/** Resolve native queue IDs and the pre-1.3 report-ID convention. */ +function flowview_schedule_queue_id(array $options) { + $value = $options['queue-id'] ?? false; + if ($value === false) { + $value = $options['report-id'] ?? false; + } + return filter_var($value, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1]]) ?: false; +} + +/** Own FlowView dispatch so failed/deferred children cannot consume queue rows. */ +function flowview_reports_run(int $id): bool { + global $config; + if (!flowview_filter_status_ready()) { return false; } - if (($report['status'] ?? '') === 'pending') { - return true; + $report = db_fetch_row_prepared('SELECT * FROM reports_queued WHERE id=? AND source=?', [$id, 'flowview']); + if (!$report) { + return false; } - $retry = $report; - $retry['id'] = 0; - $retry['status'] = 'pending'; - unset($retry['start_time']); - if ((int) sql_save($retry, 'reports_queued') <= 0) { - cacti_log('Unable to preserve the queued FlowView occurrence for retry.', false, 'FLOWVIEW'); + require_once $config['base_path'] . '/lib/poller.php'; + if (!db_execute_prepared('UPDATE reports_queued SET status=?, start_time=? WHERE id=? AND source=?', ['running', date('Y-m-d H:i:s'), $id, 'flowview'])) { return false; } - return true; + $output = []; + $status = 1; + $command = $report['run_command'] . ' --report-id=' . (int) $report['source_id'] . ' --queue-id=' . $id; + $timeout = (int) $report['run_timeout']; + if ($timeout <= 0) { + $timeout = max(1, (int) read_config_option('scheduler_timeout') ?: 300); + } + $result = exec_with_timeout($command, $output, $status, $timeout); + if ($result === false || $status !== 0) { + // Keep the original even if resetting its state fails; never delete it. + if (!db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=? AND source=?', ['pending', $id, 'flowview'])) { + cacti_log('Unable to reset the retained FlowView occurrence; queue repair is required.', false, 'FLOWVIEW'); + } + return false; + } + return db_execute_prepared('DELETE FROM reports_queued WHERE id=? AND source=?', [$id, 'flowview']) !== false; } diff --git a/run_schedule.php b/run_schedule.php index cafbdc3..44213d5 100644 --- a/run_schedule.php +++ b/run_schedule.php @@ -43,11 +43,13 @@ $force = false; $scheduled = false; $report_id = false; +$queue_id = false; $shortopts = 'VvHh'; $longopts = [ 'scheduled::', 'report-id::', + 'queue-id::', 'debug', 'force', 'version', @@ -73,6 +75,9 @@ case 'report-id': $report_id = is_array($value) ? end($value) : $value; + break; + case 'queue-id': + $queue_id = is_array($value) ? end($value) : $value; break; case 'version': display_version(); @@ -131,7 +136,7 @@ ['flowview', 'pending']); if (cacti_sizeof($report)) { - if (!flowview_filter_status_ready() || reports_run($report['id']) === false) { + if (!flowview_filter_status_ready() || flowview_reports_run($report['id']) === false) { break; } @@ -152,6 +157,7 @@ unregister_process('flowsched', 'master', 0); } } else { + $report_id = flowview_schedule_queue_id(['report-id' => $report_id, 'queue-id' => $queue_id]); if ($report_id === false) { cacti_log('FATAL: Flowview Schedule Report ID not provided', true, 'FLOWVIEW'); exit(1); @@ -167,9 +173,7 @@ $id = $report['source_id']; if (!flowview_filter_status_ready()) { - // Core queue runners may delete the claimed row after this child exits. - // Preserve a new pending row first, without touching schedule timestamps. - flowview_retry_queued_report($report); + // The FlowView parent retains this original occurrence on a retry exit. exit(75); } diff --git a/tests/Unit/FilterScheduleRetryTest.php b/tests/Unit/FilterScheduleRetryTest.php index edbfcb2..b893c83 100644 --- a/tests/Unit/FilterScheduleRetryTest.php +++ b/tests/Unit/FilterScheduleRetryTest.php @@ -1,52 +1,126 @@ 51, 'source' => 'flowview']); - expect(reports_run(51))->toBeFalse(); - expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('db_execute_prepared', 'exec_with_timeout'); - expect($GLOBALS['__test_saved_rows'])->toBe([]); -}); -it('preserves an already claimed occurrence under a new pending queue id', function () { - $original = ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'status' => 'running', 'start_time' => '2026-10-18 00:00:00', 'scheduled_time' => '2026-10-17 23:00:00', 'run_command' => 'php run_schedule.php', 'notification' => '{}']; - plugin_test_queue_db_result('sql_save', 72); - expect(flowview_retry_queued_report($original))->toBeTrue(); - $saved = $GLOBALS['__test_saved_rows'][0]; - expect($saved['table'])->toBe('reports_queued')->and($saved['row']['id'])->toBe(0)->and($saved['row']['status'])->toBe('pending'); - expect($saved['row'])->not->toHaveKey('start_time'); - foreach (['source_id', 'scheduled_time', 'run_command', 'notification'] as $key) { - expect($saved['row'][$key])->toBe($original[$key]); +it('resolves native queue IDs separately from schedule IDs and accepts legacy dispatch', function ($options, $expected) { + expect(flowview_schedule_queue_id($options))->toBe($expected); +})->with([ + [['report-id' => 7, 'queue-id' => 51], 51], [['report-id' => 51], 51], + [['report-id' => 7, 'queue-id' => false], 7], [['report-id' => 0], false], + [['report-id' => 7, 'queue-id' => 'invalid'], false], [[], false], +]); +it('keeps a pending occurrence untouched before either runner claims it', function ($legacy) { + if ($legacy) { + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview']); } - expect($GLOBALS['__test_db_calls'])->toBe([]); -}); -it('does not duplicate a row which is already pending', function () { - expect(flowview_retry_queued_report(['id' => 51, 'source' => 'flowview', 'status' => 'pending']))->toBeTrue(); - expect($GLOBALS['__test_saved_rows'])->toBe([]); -}); -it('rejects unrelated or unidentified queued rows', function ($row) { - expect(flowview_retry_queued_report($row))->toBeFalse(); - expect($GLOBALS['__test_saved_rows'])->toBe([]); -})->with([[[]], [['id' => 51, 'source' => 'other']]]); -it('reports a failed preservation instead of claiming a successful retry', function () { - plugin_test_queue_db_result('sql_save', false); - expect(flowview_retry_queued_report(['id' => 51, 'source' => 'flowview', 'status' => 'running']))->toBeFalse(); -}); -it('continues normal pre-1.3 queue processing once the schema is ready', function () { + expect($legacy ? reports_run(51) : flowview_reports_run(51))->toBeFalse(); + expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('db_execute_prepared', 'exec_with_timeout'); +})->with([false, true]); +it('retains the original on deferred or failed children and only deletes successful occurrences', function ($status, $reset, $legacy, $startup) { global $config; $old = $config['base_path']; $root = sys_get_temp_dir() . '/flowview-shim-' . bin2hex(random_bytes(8)); mkdir($root . '/lib', 0700, true); - file_put_contents($root . '/lib/poller.php', ' 51, 'source' => 'flowview', 'name' => 'Fixture', 'source_id' => 7, 'run_command' => 'fixture-command', 'run_timeout' => 60]); plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); - expect(reports_run(51))->toBeTrue(); - $sql = implode(' ', array_column($GLOBALS['__test_db_calls'], 'sql')); - expect($sql)->toContain('UPDATE reports_queued', 'DELETE FROM reports_queued'); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'name' => 'Fixture', 'source_id' => 7, 'run_command' => 'fixture-command', 'run_timeout' => $status === 1 ? 0 : 60]); + plugin_test_queue_db_result('exec_with_timeout', $status); + plugin_test_queue_db_result('exec_with_timeout_result', $startup ? false : null); + plugin_test_queue_db_result('db_execute_prepared', true); + plugin_test_queue_db_result('db_execute_prepared', $reset); + $success = $status === 0 && !$startup && $reset; + expect($legacy ? reports_run(51) : flowview_reports_run(51))->toBe($success); + $calls = $GLOBALS['__test_db_calls']; + $sql = implode(' ', array_column($calls, 'sql')); + if ($status !== 0 || $startup) { + expect($sql)->not->toContain('DELETE FROM reports_queued'); + expect($sql)->toContain('UPDATE reports_queued SET status'); + } else { + expect($sql)->toContain('DELETE FROM reports_queued'); + } + if (!$legacy) { + $exec = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'exec_with_timeout'))[0]; + expect($exec['params'][0])->toContain('--report-id=7 --queue-id=51'); + expect($exec['params'][1])->toBe($status === 1 ? 300 : 60); + } + expect($GLOBALS['__test_saved_rows'])->toBe([]); } finally { $config['base_path'] = $old; unlink($root . '/lib/poller.php'); rmdir($root . '/lib'); rmdir($root); } -}); +})->with([[75, true, false, false], [75, false, false, false], [1, true, false, false], [0, true, false, true], [0, true, false, false], [0, false, false, false], [75, true, true, false], [0, true, true, false]]); +it('rejects missing queue rows or failed claims without running a child', function ($missing) { + global $config; + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', $missing ? [] : ['id' => 51, 'source' => 'flowview']); + if (!$missing) { + // A previously included harmless fixture path satisfies require_once. + $old = $config['base_path']; + $root = sys_get_temp_dir() . '/flowview-claim-' . bin2hex(random_bytes(8)); + mkdir($root . '/lib', 0700, true); + file_put_contents($root . '/lib/poller.php', 'toBeFalse(); + expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('exec_with_timeout'); + } finally { + if (!$missing) { + $config['base_path'] = $old; + unlink($root . '/lib/poller.php'); + rmdir($root . '/lib'); + rmdir($root); + } + } +})->with([true, false]); + +it('runs the actual child entrypoint with native and legacy dispatch IDs', function ($native) { + $root = sys_get_temp_dir() . '/flowview-entrypoint-' . bin2hex(random_bytes(8)); + mkdir($root . '/plugins/flowview/includes', 0700, true); + mkdir($root . '/include'); + mkdir($root . '/lib'); + $plugin = dirname(__DIR__, 2); + copy($plugin . '/run_schedule.php', $root . '/plugins/flowview/run_schedule.php'); + foreach (['database.php', 'functions.php'] as $file) { + symlink($plugin . '/includes/' . $file, $root . '/plugins/flowview/includes/' . $file); + } + file_put_contents($root . '/plugins/flowview/setup.php', '51,"source"=>"flowview","source_id"=>7]); register_shutdown_function(function(){file_put_contents(' . $result . ',json_encode($GLOBALS["__test_db_calls"]));});'); + try { + $command = [PHP_BINARY, $root . '/plugins/flowview/run_schedule.php', '--report-id=' . ($native ? 7 : 51)]; + if ($native) { + $command[] = '--queue-id=51'; + } + $process = proc_open($command, [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes); + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + $error = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + $exit = proc_close($process); + expect($error)->toBe('')->and($output)->toBe('')->and($exit)->toBe(75); + $calls = json_decode(file_get_contents($root . '/result.json'), true); + expect($calls)->toHaveCount(2)->and($calls[0]['params'])->toBe(['flowview', 51]); + expect(array_column($calls, 'fn'))->not->toContain('db_execute_prepared'); + } finally { + $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($root, FilesystemIterator::SKIP_DOTS), RecursiveIteratorIterator::CHILD_FIRST); + foreach ($iterator as $file) { + if ($file->isDir() && !$file->isLink()) { + rmdir($file->getPathname()); + } else { + unlink($file->getPathname()); + } + } + rmdir($root); + } +})->with([false, true]); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 3854542..c55b3cc 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -132,7 +132,7 @@ function db_execute($sql, $log = true, $conn = false) { if (!function_exists('db_execute_prepared')) { function db_execute_prepared($sql, $params = array(), $log = true, $conn = false) { $GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute_prepared', 'sql' => $sql, 'params' => $params, 'conn' => $conn); - return true; + return plugin_test_db_result('db_execute_prepared', true); } } From 5fff8c95f0b3ca09f5f1cf45467b53bd63c8579a Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 18 Oct 2026 23:32:50 +1030 Subject: [PATCH 09/29] Keep failed schedules from starving healthy queue entries --- README.md | 2 + includes/functions.php | 28 +++++++++++++ run_schedule.php | 35 +--------------- tests/Unit/FilterQueueBatchTest.php | 65 +++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 34 deletions(-) create mode 100644 tests/Unit/FilterQueueBatchTest.php diff --git a/README.md b/README.md index 1035367..89c71eb 100644 --- a/README.md +++ b/README.md @@ -219,3 +219,5 @@ Filters and schedules show Enabled in green and Disabled in grey, using the Cact Missing filter-state columns are repaired by the background upgrade process, including same-version repairs. Pages return a retry message until repair completes; scheduled reports defer without issuing schema changes. Editing a schedule retains its existing disabled filter and labels it Disabled. Pending filter-state migration leaves scheduled queue entries and lastsent/start timestamps unchanged. FlowView uses its own dispatcher on both Cacti versions: deferred or failed children retain their original occurrence, without saving a replacement. Native queue IDs and the older report-ID convention are both accepted. If resetting the retained row to pending fails, the row is kept and a queue-repair error is logged. + +Queue dispatch processes a bounded snapshot, attempting each occurrence once per pass. A failed occurrence stays retained while healthy entries continue; previously attempted rows sort behind new work so repeated failures cannot monopolize the next snapshot. Schema loss or exhausted concurrency stops the pass. diff --git a/includes/functions.php b/includes/functions.php index 72f2852..b0afa7b 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -7415,3 +7415,31 @@ function flowview_reports_run(int $id): bool { } return db_execute_prepared('DELETE FROM reports_queued WHERE id=? AND source=?', [$id, 'flowview']) !== false; } + +/** Attempt a bounded, fair queue snapshot without retrying failures in the same pass. */ +function flowview_reports_run_pending(int $concurrency = 1, int $limit = 100): int { + if (!flowview_filter_status_ready()) { + return 0; + } + // Previously attempted rows sort behind new work, so a full failed batch + // cannot indefinitely starve occurrences beyond this snapshot's limit. + $reports = db_fetch_assoc_prepared('SELECT id FROM reports_queued WHERE source=? AND status=? ORDER BY start_time,scheduled_time,id LIMIT ' . max(1, min(1000, $limit)), ['flowview', 'pending']); + if (!is_array($reports)) { + cacti_log('Unable to read pending FlowView occurrences.', false, 'FLOWVIEW'); + return 0; + } + $done = 0; + foreach ($reports as $report) { + if (!flowview_filter_status_ready()) { + break; + } + $running = db_fetch_cell_prepared('SELECT COUNT(*) FROM processes WHERE tasktype=? AND taskname LIKE ?', ['flowsched', 'child%']); + if ($running === false || (int) $running >= max(1, $concurrency)) { + break; + } + if (flowview_reports_run((int) $report['id'])) { + $done++; + } + } + return $done; +} diff --git a/run_schedule.php b/run_schedule.php index 44213d5..af5249c 100644 --- a/run_schedule.php +++ b/run_schedule.php @@ -114,40 +114,7 @@ $concurrent_processes = 1; } - $reports = db_fetch_assoc_prepared('SELECT * - FROM reports_queued - WHERE status = ? - AND source = ?', - ['pending', 'flowview']); - - while(true) { - $running = db_fetch_cell_prepared('SELECT COUNT(*) - FROM processes - WHERE taskname = ? - AND tasktype LIKE ?', - ['flowsched', 'child%']); - - if ($running < $concurrent_processes) { - $report = db_fetch_row_prepared('SELECT * - FROM reports_queued - WHERE source = ? - AND status = ? - LIMIT 1', - ['flowview', 'pending']); - - if (cacti_sizeof($report)) { - if (!flowview_filter_status_ready() || flowview_reports_run($report['id']) === false) { - break; - } - - $run++; - } else { - break; - } - - sleep(1); - } - } + $run = flowview_reports_run_pending((int) $concurrent_processes); $end = microtime(true); diff --git a/tests/Unit/FilterQueueBatchTest.php b/tests/Unit/FilterQueueBatchTest.php new file mode 100644 index 0000000..49959c2 --- /dev/null +++ b/tests/Unit/FilterQueueBatchTest.php @@ -0,0 +1,65 @@ +oldBase = $config['base_path']; + $this->root = sys_get_temp_dir() . '/flowview-batch-' . bin2hex(random_bytes(8)); + mkdir($this->root . '/lib', 0700, true); + file_put_contents($this->root . '/lib/poller.php', 'root; +}); +afterEach(function () { + global $config; + $config['base_path'] = $this->oldBase; + unlink($this->root . '/lib/poller.php'); + rmdir($this->root . '/lib'); + rmdir($this->root); +}); +it('attempts a failed occurrence once and still dispatches the healthy row behind it', function () { + for ($i = 0;$i < 5;$i++) { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + } + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 51], ['id' => 52]]); + foreach ([51, 52] as $id) { + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => $id, 'source' => 'flowview', 'source_id' => $id - 44, 'run_command' => 'fixture', 'run_timeout' => 60]); + } + plugin_test_queue_db_result('exec_with_timeout', 75); + plugin_test_queue_db_result('exec_with_timeout', 0); + expect(flowview_reports_run_pending())->toBe(1); + $calls = $GLOBALS['__test_db_calls']; + $exec = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'exec_with_timeout')); + expect($exec)->toHaveCount(2)->and($exec[0]['params'][0])->toContain('--queue-id=51')->and($exec[1]['params'][0])->toContain('--queue-id=52'); + $deletes = array_values(array_filter($calls, fn ($c) => str_starts_with($c['sql'], 'DELETE FROM reports_queued'))); + expect($deletes)->toHaveCount(1)->and($deletes[0]['params'])->toBe([52, 'flowview']); + $query = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'db_fetch_assoc_prepared'))[0]; + expect($query['sql'])->toContain('ORDER BY start_time,scheduled_time,id LIMIT 100'); +}); +it('stops the batch when schema readiness is lost instead of starting another child', function () { + foreach ([true, true, true, false] as $ready) { + plugin_test_queue_db_result('db_fetch_row', $ready ? ['Field' => 'enabled'] : []); + } + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 51], ['id' => 52]]); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); + plugin_test_queue_db_result('exec_with_timeout', 75); + expect(flowview_reports_run_pending())->toBe(0); + expect(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'exec_with_timeout'))->toHaveCount(1); +}); +it('does not dispatch beyond capacity or after a failed process count', function ($running) { + for ($i = 0;$i < 2;$i++) { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + } + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 51]]); + plugin_test_queue_db_result('db_fetch_cell_prepared', $running); + expect(flowview_reports_run_pending(2))->toBe(0); + expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('exec_with_timeout'); +})->with([2, false]); +it('does not read a queue before schema readiness and does not ignore failed queue reads', function () { + expect(flowview_reports_run_pending())->toBe(0); + plugin_test_reset(); + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_assoc_prepared', false); + expect(flowview_reports_run_pending(1, 100000))->toBe(0); + $query = end($GLOBALS['__test_db_calls']); + expect($query['sql'])->toContain('LIMIT 1000'); +}); From 3ced8529701dd9667207f4c12d33e00f26416a4e Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 18 Oct 2026 23:56:14 +1030 Subject: [PATCH 10/29] Recover dead scheduler registrations without removing live children --- README.md | 2 ++ includes/functions-pre13.php | 4 ++- includes/functions.php | 37 ++++++++++++++++++-- tests/Unit/FilterQueueBatchTest.php | 47 +++++++++++++++++++++++++- tests/Unit/FilterScheduleRetryTest.php | 10 ++++-- 5 files changed, 94 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 89c71eb..222af62 100644 --- a/README.md +++ b/README.md @@ -221,3 +221,5 @@ Missing filter-state columns are repaired by the background upgrade process, inc Pending filter-state migration leaves scheduled queue entries and lastsent/start timestamps unchanged. FlowView uses its own dispatcher on both Cacti versions: deferred or failed children retain their original occurrence, without saving a replacement. Native queue IDs and the older report-ID convention are both accepted. If resetting the retained row to pending fails, the row is kept and a queue-repair error is logged. Queue dispatch processes a bounded snapshot, attempting each occurrence once per pass. A failed occurrence stays retained while healthy entries continue; previously attempted rows sort behind new work so repeated failures cannot monopolize the next snapshot. Schema loss or exhausted concurrency stops the pass. + +Scheduler capacity checks remove exact child registrations only when the operating system confirms that their PID no longer exists. Live or uncertain processes (including permission errors and platforms without POSIX liveness probes) retain their capacity reservation. Failed legacy queue resets now log an explicit queue-repair error while retaining the original occurrence. diff --git a/includes/functions-pre13.php b/includes/functions-pre13.php index 48562c7..1355c69 100644 --- a/includes/functions-pre13.php +++ b/includes/functions-pre13.php @@ -298,7 +298,9 @@ function reports_run($id): bool { cacti_log($stats, false, 'SYSTEM'); if ($report['source'] === 'flowview' && ($last_line === false || $return_code !== 0)) { - db_execute_prepared('UPDATE reports_queued SET status = ? WHERE id = ?', ['pending', $id]); + if (!db_execute_prepared('UPDATE reports_queued SET status = ? WHERE id = ?', ['pending', $id])) { + cacti_log('Unable to reset the retained FlowView occurrence; queue repair is required.', false, 'FLOWVIEW'); + } return false; } db_execute_prepared('DELETE FROM reports_queued WHERE id = ?', [$id]); diff --git a/includes/functions.php b/includes/functions.php index b0afa7b..db81890 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -7433,8 +7433,8 @@ function flowview_reports_run_pending(int $concurrency = 1, int $limit = 100): i if (!flowview_filter_status_ready()) { break; } - $running = db_fetch_cell_prepared('SELECT COUNT(*) FROM processes WHERE tasktype=? AND taskname LIKE ?', ['flowsched', 'child%']); - if ($running === false || (int) $running >= max(1, $concurrency)) { + $running = flowview_running_schedule_children(); + if ($running === null || (int) $running >= max(1, $concurrency)) { break; } if (flowview_reports_run((int) $report['id'])) { @@ -7443,3 +7443,36 @@ function flowview_reports_run_pending(int $concurrency = 1, int $limit = 100): i } return $done; } + +/** Count live/uncertain children, removing only exact registrations confirmed dead. */ +function flowview_running_schedule_children(): ?int { + $rows = db_fetch_assoc_prepared('SELECT taskname,taskid,pid FROM processes WHERE tasktype=? AND taskname LIKE ?', ['flowsched', 'child%']); + if (!is_array($rows)) { + return null; + } + $running = 0; + foreach ($rows as $row) { + if (flowview_schedule_pid_alive($row['pid']) !== false) { + $running++; + continue; + } + // A concurrent registration with another PID must survive this cleanup. + if (!db_execute_prepared('DELETE FROM processes WHERE tasktype=? AND taskname=? AND taskid=? AND pid=?', ['flowsched', $row['taskname'], $row['taskid'], $row['pid']])) { + $running++; + cacti_log('Unable to remove a dead FlowView child registration.', false, 'FLOWVIEW'); + } + } + return $running; +} + +/** Null means liveness cannot be established; permission errors never mean dead. */ +function flowview_schedule_pid_alive($pid): ?bool { + $valid = filter_var($pid, FILTER_VALIDATE_INT, ['options' => ['min_range' => 2, 'max_range' => 2147483647]]); + if (!$valid || !function_exists('posix_kill') || !function_exists('posix_get_last_error')) { + return null; + } + if (@posix_kill($valid, 0)) { + return true; + } + return posix_get_last_error() === 3 ? false : null; +} diff --git a/tests/Unit/FilterQueueBatchTest.php b/tests/Unit/FilterQueueBatchTest.php index 49959c2..eb0f7d6 100644 --- a/tests/Unit/FilterQueueBatchTest.php +++ b/tests/Unit/FilterQueueBatchTest.php @@ -50,7 +50,7 @@ plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); } plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 51]]); - plugin_test_queue_db_result('db_fetch_cell_prepared', $running); + plugin_test_queue_db_result('db_fetch_assoc_prepared', $running === false ? false : array_fill(0, $running, ['taskname' => 'child_7', 'taskid' => 0, 'pid' => getmypid()])); expect(flowview_reports_run_pending(2))->toBe(0); expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('exec_with_timeout'); })->with([2, false]); @@ -63,3 +63,48 @@ $query = end($GLOBALS['__test_db_calls']); expect($query['sql'])->toContain('LIMIT 1000'); }); + +it('reclaims a failed childs dead registration before dispatching the healthy occurrence', function () { + if (!function_exists('posix_kill')) { + $this->markTestSkipped('POSIX liveness is unavailable'); + } + for ($i = 0;$i < 5;$i++) { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + } + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 51], ['id' => 52]]); + plugin_test_queue_db_result('db_fetch_assoc_prepared', []); + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['taskname' => 'child_7', 'taskid' => 0, 'pid' => 2147483647]]); + foreach ([51, 52] as $id) { + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => $id, 'source' => 'flowview', 'source_id' => $id - 44, 'run_command' => 'fixture', 'run_timeout' => 60]); + } + plugin_test_queue_db_result('exec_with_timeout', 75); + plugin_test_queue_db_result('exec_with_timeout', 0); + expect(flowview_reports_run_pending())->toBe(1); + $calls = $GLOBALS['__test_db_calls']; + $deletes = array_values(array_filter($calls, fn ($c) => str_starts_with($c['sql'], 'DELETE FROM processes'))); + expect($deletes)->toHaveCount(1)->and($deletes[0]['params'])->toBe(['flowsched', 'child_7', 0, 2147483647]); + expect(array_filter($calls, fn ($c) => $c['fn'] === 'exec_with_timeout'))->toHaveCount(2); +}); +it('keeps live and uncertain child registrations and removes only confirmed dead ones', function () { + if (!function_exists('posix_kill')) { + $this->markTestSkipped('POSIX liveness is unavailable'); + } + plugin_test_queue_db_result('db_fetch_assoc_prepared', [ + ['taskname' => 'child_7', 'taskid' => 0, 'pid' => 2147483647], + ['taskname' => 'child_8', 'taskid' => 0, 'pid' => getmypid()], + ['taskname' => 'child_9', 'taskid' => 0, 'pid' => 0], + ]); + expect(flowview_running_schedule_children())->toBe(2); + expect(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE')))->toHaveCount(1); + expect(flowview_schedule_pid_alive(getmypid()))->toBeTrue()->and(flowview_schedule_pid_alive(0))->toBeNull(); +}); +it('does not grant capacity when removal of a dead registration fails', function () { + if (!function_exists('posix_kill')) { + $this->markTestSkipped('POSIX liveness is unavailable'); + } + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['taskname' => 'child_7', 'taskid' => 0, 'pid' => 2147483647]]); + plugin_test_queue_db_result('db_execute_prepared', false); + $GLOBALS['__test_cacti_log'] = []; + expect(flowview_running_schedule_children())->toBe(1); + expect(implode(' ', $GLOBALS['__test_cacti_log']))->toContain('Unable to remove a dead FlowView child registration'); +}); diff --git a/tests/Unit/FilterScheduleRetryTest.php b/tests/Unit/FilterScheduleRetryTest.php index b893c83..2668c39 100644 --- a/tests/Unit/FilterScheduleRetryTest.php +++ b/tests/Unit/FilterScheduleRetryTest.php @@ -1,6 +1,9 @@ toBe($expected); })->with([ @@ -34,6 +37,9 @@ $calls = $GLOBALS['__test_db_calls']; $sql = implode(' ', array_column($calls, 'sql')); if ($status !== 0 || $startup) { + if (!$reset) { + expect(implode(' ', $GLOBALS['__test_cacti_log']))->toContain('queue repair is required'); + } expect($sql)->not->toContain('DELETE FROM reports_queued'); expect($sql)->toContain('UPDATE reports_queued SET status'); } else { @@ -51,7 +57,7 @@ rmdir($root . '/lib'); rmdir($root); } -})->with([[75, true, false, false], [75, false, false, false], [1, true, false, false], [0, true, false, true], [0, true, false, false], [0, false, false, false], [75, true, true, false], [0, true, true, false]]); +})->with([[75, true, false, false], [75, false, false, false], [1, true, false, false], [0, true, false, true], [0, true, false, false], [0, false, false, false], [75, true, true, false], [75, false, true, false], [0, true, true, false]]); it('rejects missing queue rows or failed claims without running a child', function ($missing) { global $config; plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); From f46b0319b63f437f7fc8672243f60e14e53d1e45 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 19 Oct 2026 00:14:28 +1030 Subject: [PATCH 11/29] Retain refused reports and concurrent child reservations --- includes/functions.php | 6 +++++- run_schedule.php | 2 +- tests/Unit/FilterQueueBatchTest.php | 16 ++++++++++++++++ tests/Unit/FilterScheduleRetryTest.php | 24 ++++++++++++++++++++---- tests/bootstrap-unit.php | 12 ++++++++++++ 5 files changed, 54 insertions(+), 6 deletions(-) diff --git a/includes/functions.php b/includes/functions.php index db81890..30ff87b 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -7457,9 +7457,13 @@ function flowview_running_schedule_children(): ?int { continue; } // A concurrent registration with another PID must survive this cleanup. - if (!db_execute_prepared('DELETE FROM processes WHERE tasktype=? AND taskname=? AND taskid=? AND pid=?', ['flowsched', $row['taskname'], $row['taskid'], $row['pid']])) { + $deleted = db_execute_prepared('DELETE FROM processes WHERE tasktype=? AND taskname=? AND taskid=? AND pid=?', ['flowsched', $row['taskname'], $row['taskid'], $row['pid']]); + if (!$deleted) { $running++; cacti_log('Unable to remove a dead FlowView child registration.', false, 'FLOWVIEW'); + } elseif ((int) db_affected_rows() !== 1) { + // A replacement PID may now own the reservation; do not grant its slot. + $running++; } } return $running; diff --git a/run_schedule.php b/run_schedule.php index af5249c..bc83cc2 100644 --- a/run_schedule.php +++ b/run_schedule.php @@ -147,7 +147,7 @@ /* silently end if the registered process is still running, or process table missing */ if (!$force) { if (!register_process_start('flowsched', "child_$id", 0, 1200)) { - exit(0); + exit(75); } } diff --git a/tests/Unit/FilterQueueBatchTest.php b/tests/Unit/FilterQueueBatchTest.php index eb0f7d6..55187b8 100644 --- a/tests/Unit/FilterQueueBatchTest.php +++ b/tests/Unit/FilterQueueBatchTest.php @@ -108,3 +108,19 @@ expect(flowview_running_schedule_children())->toBe(1); expect(implode(' ', $GLOBALS['__test_cacti_log']))->toContain('Unable to remove a dead FlowView child registration'); }); + +it('keeps a capacity reservation when a concurrent replacement makes dead-PID deletion affect no rows', function () { + if (!function_exists('posix_kill')) { + $this->markTestSkipped('POSIX liveness is unavailable'); + } + for ($i = 0;$i < 2;$i++) { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + } + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 52]]); + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['taskname' => 'child_7', 'taskid' => 0, 'pid' => 2147483647]]); + plugin_test_queue_db_result('db_affected_rows', 0); + expect(flowview_reports_run_pending(1))->toBe(0); + expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('exec_with_timeout'); + $deletes = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM processes'))); + expect($deletes)->toHaveCount(1)->and($deletes[0]['params'])->toBe(['flowsched', 'child_7', 0, 2147483647]); +}); diff --git a/tests/Unit/FilterScheduleRetryTest.php b/tests/Unit/FilterScheduleRetryTest.php index 2668c39..98a5198 100644 --- a/tests/Unit/FilterScheduleRetryTest.php +++ b/tests/Unit/FilterScheduleRetryTest.php @@ -84,7 +84,7 @@ } })->with([true, false]); -it('runs the actual child entrypoint with native and legacy dispatch IDs', function ($native) { +it('runs the actual child entrypoint with native and legacy dispatch IDs', function ($native, $ready) { $root = sys_get_temp_dir() . '/flowview-entrypoint-' . bin2hex(random_bytes(8)); mkdir($root . '/plugins/flowview/includes', 0700, true); mkdir($root . '/include'); @@ -101,7 +101,8 @@ $bootstrap = var_export(dirname(__DIR__) . '/bootstrap-unit.php', true); $result = var_export($root . '/result.json', true); $core = var_export($root, true); - file_put_contents($root . '/include/cli_check.php', '51,"source"=>"flowview","source_id"=>7]); register_shutdown_function(function(){file_put_contents(' . $result . ',json_encode($GLOBALS["__test_db_calls"]));});'); + $readyCode = $ready ? 'plugin_test_queue_db_result("db_fetch_row", ["Field"=>"enabled"]);plugin_test_queue_db_result("register_process_start", false);' : ''; + file_put_contents($root . '/include/cli_check.php', '51,"source"=>"flowview","source_id"=>7]); register_shutdown_function(function(){file_put_contents(' . $result . ',json_encode($GLOBALS["__test_db_calls"]));});'); try { $command = [PHP_BINARY, $root . '/plugins/flowview/run_schedule.php', '--report-id=' . ($native ? 7 : 51)]; if ($native) { @@ -116,8 +117,23 @@ $exit = proc_close($process); expect($error)->toBe('')->and($output)->toBe('')->and($exit)->toBe(75); $calls = json_decode(file_get_contents($root . '/result.json'), true); - expect($calls)->toHaveCount(2)->and($calls[0]['params'])->toBe(['flowview', 51]); + expect($calls)->toHaveCount($ready ? 3 : 2)->and($calls[0]['params'])->toBe(['flowview', 51]); expect(array_column($calls, 'fn'))->not->toContain('db_execute_prepared'); + if ($ready) { + global $config; + $oldBase = $config['base_path']; + $config['base_path'] = $root; + try { + plugin_test_reset(); + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); + plugin_test_queue_db_result('exec_with_timeout', $exit); + expect(flowview_reports_run(51))->toBeFalse(); + expect(implode(' ', array_column($GLOBALS['__test_db_calls'], 'sql')))->not->toContain('DELETE FROM reports_queued'); + } finally { + $config['base_path'] = $oldBase; + } + } } finally { $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($root, FilesystemIterator::SKIP_DOTS), RecursiveIteratorIterator::CHILD_FIRST); foreach ($iterator as $file) { @@ -129,4 +145,4 @@ } rmdir($root); } -})->with([false, true]); +})->with([[false, false], [true, false], [false, true], [true, true]]); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index c55b3cc..8bf3121 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -497,3 +497,15 @@ function exec_with_timeout($command, &$output, &$status, $timeout = 5) { return plugin_test_db_result('exec_with_timeout_result', null); } } + +if (!function_exists('db_affected_rows')) { + function db_affected_rows($conn = false) { + return plugin_test_db_result('db_affected_rows', 1); + } +} +if (!function_exists('register_process_start')) { + function register_process_start($type, $name, $id = 0, $timeout = 300) { + $GLOBALS['__test_db_calls'][] = ['fn' => 'register_process_start', 'sql' => '', 'params' => [$type, $name, $id, $timeout]]; + return plugin_test_db_result('register_process_start', true); + } +} From ca7e07f67ed56d4af958d5f6797ca797465d7fd2 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 19 Oct 2026 13:09:25 +1030 Subject: [PATCH 12/29] test: verify queue fairness and background filter repair --- tests/README.md | 8 ++ tests/Unit/FilterLifecycleRegressionTest.php | 78 ++++++++++++++++++++ tests/fixtures/filter-lifecycle.php | 57 ++++++++++++++ tests/fixtures/filter-queue.php | 49 ++++++++++++ 4 files changed, 192 insertions(+) create mode 100644 tests/Unit/FilterLifecycleRegressionTest.php create mode 100644 tests/fixtures/filter-lifecycle.php create mode 100644 tests/fixtures/filter-queue.php diff --git a/tests/README.md b/tests/README.md index e938b89..35d6c49 100644 --- a/tests/README.md +++ b/tests/README.md @@ -38,3 +38,11 @@ connection assertions. Security tests validate PHP 8.2 syntax compatibility and the plugin's install/version/uninstall structure without executing plugin source. +The filter lifecycle regressions also launch the actual web and upgrade entrypoints +in subprocesses with isolated core API fixtures. They verify that all three web +pages defer schema repair, including same-version installations, and that the +background worker registers before attempting repair. The queue regression uses +an in-memory SQLite database to check persisted ordering across multiple passes: +a full failed batch remains pending while healthy rows advance on the next pass. +This queue test requires PDO SQLite and skips explicitly when it is unavailable; +it does not validate MariaDB schema syntax or metadata locking. diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php new file mode 100644 index 0000000..cb46c40 --- /dev/null +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -0,0 +1,78 @@ +fixtureRoot = sys_get_temp_dir() . '/flowview-lifecycle-' . bin2hex(random_bytes(8)); + mkdir($this->fixtureRoot . '/plugins/flowview', 0700, true); + mkdir($this->fixtureRoot . '/include'); + mkdir($this->fixtureRoot . '/lib'); + $plugin = dirname(__DIR__, 2); + foreach (['setup.php', 'includes', 'flowview.php', 'flowview_filters.php', 'flowview_schedules.php'] as $file) { + symlink($plugin . '/' . $file, $this->fixtureRoot . '/plugins/flowview/' . $file); + } + copy($plugin . '/flowview_upgrade.php', $this->fixtureRoot . '/plugins/flowview/flowview_upgrade.php'); + file_put_contents($this->fixtureRoot . '/plugins/flowview/INFO', "[info]\nversion=fixture-version\n"); + foreach (['auth.php', 'cli_check.php'] as $file) { + copy(dirname(__DIR__) . '/fixtures/filter-lifecycle.php', $this->fixtureRoot . '/include/' . $file); + } + foreach (['poller.php', 'time.php', 'timespan_settings.php'] as $file) { + file_put_contents($this->fixtureRoot . '/lib/' . $file, 'runFixture = function ($command, $mode = 'pending') { + $env = getenv(); + $env['FLOWVIEW_FIXTURE_MODE'] = $mode; + $env['FLOWVIEW_FIXTURE_RESULT'] = $this->fixtureRoot . '/result.json'; + $process = proc_open($command, [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes, $this->fixtureRoot . '/plugins/flowview', $env); + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + $error = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + $exit = proc_close($process); + expect($error)->toBe(''); + return [$exit, $output]; + }; +}); +afterEach(function () { + $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($this->fixtureRoot, FilesystemIterator::SKIP_DOTS), RecursiveIteratorIterator::CHILD_FIRST); + foreach ($iterator as $file) { + if ($file->isDir() && !$file->isLink()) { + rmdir($file->getPathname()); + } else { + unlink($file->getPathname()); + } + } + rmdir($this->fixtureRoot); +}); +it('serves a retry response without DDL from every guarded web entrypoint', function ($page, $ready) { + [$exit, $output] = ($this->runFixture)([PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/' . $page], $ready ? 'ready' : 'pending'); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + $actions = array_column($result['calls'], 0); + expect($exit)->toBe(0)->and($actions)->not->toContain('ddl'); + if ($ready) { + expect($output)->toBe('')->and($actions)->toContain('page-ready')->not->toContain('background'); + } else { + expect($result['status'])->toBe(503)->and($output)->toContain('schema upgrade is pending') + ->and($actions)->not->toContain('page-ready'); + $background = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'background')); + expect($background)->toHaveCount(1)->and($background[0][1][1])->toContain('/plugins/flowview/flowview_upgrade.php'); + } +})->with(['flowview.php', 'flowview_filters.php', 'flowview_schedules.php'])->with([false, true]); +it('repairs same-version schemas only after background registration and reports repair failure', function ($mode, $code, $writes) { + [$exit, $output] = ($this->runFixture)([PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/flowview_upgrade.php'], $mode); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + expect($result)->toBeArray($output); + $actions = array_column($result['calls'], 0); + expect($exit)->toBe($code)->and($actions[0])->toBe('register') + ->and(array_filter($actions, fn ($action) => $action === 'ddl'))->toHaveCount($writes); +})->with([['pending', 0, 1], ['failed', 1, 1], ['ready', 0, 0], ['busy', 1, 0]]); +it('retains a full failing snapshot while advancing to healthy rows on the next pass', function () { + if (!extension_loaded('pdo_sqlite')) { + $this->markTestSkipped('PDO SQLite is needed for the persisted queue regression'); + } + [$exit, $output] = ($this->runFixture)([PHP_BINARY, dirname(__DIR__) . '/fixtures/filter-queue.php', dirname(__DIR__, 2), $this->fixtureRoot]); + $result = json_decode($output, true); + expect($exit)->toBe(0)->and($result['first'])->toBe(0)->and($result['second'])->toBe(2)->and($result['third'])->toBe(0) + ->and($result['attempts'])->toBe([1, 2, 3, 4, 1, 2]) + ->and($result['afterFirst'])->toHaveCount(4) + ->and($result['remaining'])->toBe([['id' => 1, 'status' => 'pending'], ['id' => 2, 'status' => 'pending']]); +}); diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php new file mode 100644 index 0000000..5b8b4d0 --- /dev/null +++ b/tests/fixtures/filter-lifecycle.php @@ -0,0 +1,57 @@ + dirname(__DIR__), 'poller_id' => 1]; +$local_db_cnn_id = 'fixture'; +$mode = getenv('FLOWVIEW_FIXTURE_MODE'); +$ready = $mode === 'ready'; +$calls = []; +const MESSAGE_LEVEL_INFO = 1; +function __(string $text, ...$args) { + return $text; +} +function html_escape($text) { + return htmlspecialchars($text, ENT_QUOTES, 'UTF-8'); +} +function cacti_sizeof($value) { + return is_array($value) ? count($value) : 0; +} +function read_config_option($name) { + return $name === 'path_php_binary' ? PHP_BINARY : ''; +} +function cacti_log(...$args) { +} +function raise_message(...$args) { +} +function db_fetch_cell_prepared(...$args) { + return 'fixture-version'; +} +function db_fetch_cell(...$args) { + return str_contains($args[0], 'SELECT version') ? 'fixture-version' : 1; +} +function db_fetch_row($sql, ...$args) { + $GLOBALS['calls'][] = ['read', $sql]; + return $GLOBALS['ready'] ? ['Field' => 'enabled'] : []; +} +function db_execute($sql, ...$args) { + $GLOBALS['calls'][] = ['ddl', $sql]; + $GLOBALS['ready'] = $GLOBALS['mode'] !== 'failed'; + return $GLOBALS['ready']; +} +function exec_background(...$args) { + $GLOBALS['calls'][] = ['background', $args]; +} +function register_process_start(...$args) { + $GLOBALS['calls'][] = ['register', $args]; + return $GLOBALS['mode'] !== 'busy'; +} +function unregister_process(...$args) { + $GLOBALS['calls'][] = ['unregister', $args]; +} +function set_default_action() { + $GLOBALS['calls'][] = ['page-ready']; + exit; +} +register_shutdown_function(function () { + file_put_contents(getenv('FLOWVIEW_FIXTURE_RESULT'), json_encode(['calls' => $GLOBALS['calls'], 'status' => http_response_code()])); +}); diff --git a/tests/fixtures/filter-queue.php b/tests/fixtures/filter-queue.php new file mode 100644 index 0000000..f5715da --- /dev/null +++ b/tests/fixtures/filter-queue.php @@ -0,0 +1,49 @@ + $argv[2]]; +$db = new PDO('sqlite::memory:'); +$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); +$db->exec('CREATE TABLE reports_queued (id INTEGER PRIMARY KEY, source TEXT, source_id INTEGER, status TEXT, start_time TEXT, scheduled_time TEXT, run_command TEXT, run_timeout INTEGER)'); +for ($id = 1; $id <= 4; $id++) { + $db->exec("INSERT INTO reports_queued VALUES ($id, 'flowview', $id, 'pending', '0000-00-00 00:00:00', '2026-01-01 00:00:00', 'fixture', 1)"); +} +$attempts = []; +function db_fetch_row(...$args) { + return ['Field' => 'enabled']; +} +function db_fetch_assoc_prepared($sql, $params = [], ...$args) { + if (str_contains($sql, 'FROM processes')) { + return []; + } + $stmt = $GLOBALS['db']->prepare($sql); + $stmt->execute($params); + return $stmt->fetchAll(PDO::FETCH_ASSOC); +} +function db_fetch_row_prepared($sql, $params = [], ...$args) { + return db_fetch_assoc_prepared($sql, $params)[0] ?? []; +} +function db_execute_prepared($sql, $params = [], ...$args) { + $stmt = $GLOBALS['db']->prepare($sql); + return $stmt->execute($params); +} +function exec_with_timeout($command, &$output, &$status, $timeout) { + preg_match('/--queue-id=(\d+)/', $command, $match); + $id = (int) $match[1]; + $GLOBALS['attempts'][] = $id; + $status = $id <= 2 ? 75 : 0; + return ''; +} +function read_config_option(...$args) { + return ''; +} +function cacti_log(...$args) { +} +require $argv[1] . '/includes/database.php'; +require $argv[1] . '/includes/functions.php'; +$first = flowview_reports_run_pending(1, 2); +$afterFirst = db_fetch_assoc_prepared('SELECT id,status FROM reports_queued ORDER BY id'); +$second = flowview_reports_run_pending(1, 2); +$remaining = db_fetch_assoc_prepared('SELECT id,status FROM reports_queued ORDER BY id'); +$third = flowview_reports_run_pending(1, 2); +echo json_encode(compact('first', 'second', 'third', 'attempts', 'afterFirst', 'remaining')); From bed400426cb5faa0404c10b208030e84e75435bf Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 19 Oct 2026 13:12:08 +1030 Subject: [PATCH 13/29] fix: retain scheduled reports after late schema readiness failure --- includes/functions.php | 9 ++--- run_schedule.php | 11 ++++-- tests/Unit/FilterLifecycleRegressionTest.php | 35 +++++++++++++++++++- tests/Unit/FilterStatusTest.php | 19 +++++++++-- tests/bootstrap-unit.php | 9 +++++ tests/fixtures/filter-lifecycle.php | 17 ++++++++-- 6 files changed, 89 insertions(+), 11 deletions(-) diff --git a/includes/functions.php b/includes/functions.php index 30ff87b..c074b5b 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -1916,13 +1916,13 @@ function get_port_name($port_num, int $port_proto = 6) { * @param mixed $id * @param mixed $report_id * - * @return void + * @return bool False when schema readiness requires retry; true otherwise. */ -function plugin_flowview_run_schedule($id, $report_id): void { +function plugin_flowview_run_schedule($id, $report_id): bool { global $config; if (!flowview_filter_status_ready()) { - return; + return false; } $start_time = microtime(true); @@ -1938,7 +1938,7 @@ function plugin_flowview_run_schedule($id, $report_id): void { [$schedule['query_id']]); if (empty($query) || ($query['enabled'] ?? '') !== 'on') { - return; + return true; } /* get the timespan from the query */ @@ -2006,6 +2006,7 @@ function plugin_flowview_run_schedule($id, $report_id): void { } else { cacti_log(sprintf('WARNING: Running of Flowview Schedule %s Returned not Data. Please Check your Flowview Schedule', $report_id), false, 'FLOWVIEW'); } + return true; } /** diff --git a/run_schedule.php b/run_schedule.php index bc83cc2..c13f28c 100644 --- a/run_schedule.php +++ b/run_schedule.php @@ -161,13 +161,20 @@ [$id]); if (cacti_sizeof($schedule)) { + if (!plugin_flowview_run_schedule($id, $report_id)) { + // A later readiness failure must preserve the original occurrence + // and its schedule timestamps just like the initial check does. + if (!$force) { + unregister_process('flowsched', "child_$id", 0); + } + exit(75); + } + flowview_db_execute_prepared('UPDATE plugin_flowview_schedules SET lastsent = ?, start = ? WHERE id = ?', array($r, date('Y-m-d H:i:s'), $id)); - plugin_flowview_run_schedule($id, $report_id); - $end = microtime(true); $cacti_stats = sprintf('Time:%01.4f Schedule:%s', round($end-$start,2), $id); diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index cb46c40..4dba2dd 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -9,7 +9,9 @@ foreach (['setup.php', 'includes', 'flowview.php', 'flowview_filters.php', 'flowview_schedules.php'] as $file) { symlink($plugin . '/' . $file, $this->fixtureRoot . '/plugins/flowview/' . $file); } - copy($plugin . '/flowview_upgrade.php', $this->fixtureRoot . '/plugins/flowview/flowview_upgrade.php'); + foreach (['flowview_upgrade.php', 'run_schedule.php'] as $file) { + copy($plugin . '/' . $file, $this->fixtureRoot . '/plugins/flowview/' . $file); + } file_put_contents($this->fixtureRoot . '/plugins/flowview/INFO', "[info]\nversion=fixture-version\n"); foreach (['auth.php', 'cli_check.php'] as $file) { copy(dirname(__DIR__) . '/fixtures/filter-lifecycle.php', $this->fixtureRoot . '/include/' . $file); @@ -76,3 +78,34 @@ ->and($result['afterFirst'])->toHaveCount(4) ->and($result['remaining'])->toBe([['id' => 1, 'status' => 'pending'], ['id' => 2, 'status' => 'pending']]); }); + +it('preserves late schema failures through the real child and parent completion path', function ($native, $retry) { + $command = [PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/run_schedule.php', '--report-id=' . ($native ? 7 : 51)]; + if ($native) { + $command[] = '--queue-id=51'; + } + [$exit, $output] = ($this->runFixture)($command, $retry ? 'late' : 'child-disabled'); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + $writes = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'write')); + $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); + expect($output)->toBe('')->and($exit)->toBe($retry ? 75 : 0) + ->and($released)->toBe([['unregister', ['flowsched', 'child_7', 0]]]) + ->and($writes)->toHaveCount($retry ? 0 : 1); + if (!$retry) { + expect($writes[0][1])->toContain('UPDATE plugin_flowview_schedules'); + } + global $config; + $oldBase = $config['base_path']; + $config['base_path'] = $this->fixtureRoot; + try { + plugin_test_reset(); + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); + plugin_test_queue_db_result('exec_with_timeout', $exit); + expect(flowview_reports_run(51))->toBe(!$retry); + $deletes = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM reports_queued'))); + expect($deletes)->toHaveCount($retry ? 0 : 1); + } finally { + $config['base_path'] = $oldBase; + } +})->with([false, true])->with([false, true]); diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index 6dbb666..8e5e3e3 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -45,7 +45,7 @@ })->with([true, false]); it('does not run a schedule when the state schema is unavailable', function () { - plugin_flowview_run_schedule(1, 1); + expect(plugin_flowview_run_schedule(1, 1))->toBeFalse(); expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('db_fetch_row_prepared'); }); @@ -54,7 +54,7 @@ plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 7]); plugin_test_queue_db_result('db_fetch_row_prepared', $query); - plugin_flowview_run_schedule(1, 1); + expect(plugin_flowview_run_schedule(1, 1))->toBeTrue(); $reads = array_filter($GLOBALS['__test_db_calls'], fn ($call) => $call['fn'] === 'db_fetch_row_prepared'); expect($reads)->toHaveCount(2); })->with([[[]], [['enabled' => '']], [['name' => 'legacy']]]); @@ -86,3 +86,18 @@ get_timespan($span, 2000, 1, 0); expect($span)->toBe(['begin_now' => 200, 'end_now' => 2000]); }); + +it('returns success after preparing an enabled schedule', function () { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 3, 'title' => 'Fixture', 'format_file' => 'default']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['enabled' => 'on', 'timespan' => 1, 'name' => 'Fixture']); + plugin_test_queue_db_result('get_timespan', ['begin_now' => 0, 'end_now' => 2000]); + set_request_var('timespan', 'custom'); + set_request_var('query', 3); + set_request_var('date1', date('Y-m-d H:i:s', 100)); + set_request_var('date2', date('Y-m-d H:i:s', 2000)); + $key = get_flowview_session_key(3, 100, 2000); + $_SESSION['sess_flowdata'][$key] = ['timeout' => time() + 60, 'data' => ['table' => 'Fixture', 'data' => [['bytes' => 42]]]]; + expect(plugin_flowview_run_schedule(7, 51))->toBeTrue(); + expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->toContain('reports_load_format_file'); +}); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 8bf3121..4aefb9c 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -509,3 +509,12 @@ function register_process_start($type, $name, $id = 0, $timeout = 300) { return plugin_test_db_result('register_process_start', true); } } + +if (!function_exists('reports_load_format_file')) { + function reports_load_format_file($format, &$output, &$report_tag, &$theme) { + $GLOBALS['__test_db_calls'][] = ['fn' => 'reports_load_format_file', 'sql' => '', 'params' => [$format]]; + $output = ''; + $report_tag = true; + return true; + } +} diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php index 5b8b4d0..2abca21 100644 --- a/tests/fixtures/filter-lifecycle.php +++ b/tests/fixtures/filter-lifecycle.php @@ -4,7 +4,8 @@ $config = ['base_path' => dirname(__DIR__), 'poller_id' => 1]; $local_db_cnn_id = 'fixture'; $mode = getenv('FLOWVIEW_FIXTURE_MODE'); -$ready = $mode === 'ready'; +$ready = in_array($mode, ['ready', 'late', 'child-disabled'], true); +$metadataReads = 0; $calls = []; const MESSAGE_LEVEL_INFO = 1; function __(string $text, ...$args) { @@ -31,7 +32,8 @@ function db_fetch_cell(...$args) { } function db_fetch_row($sql, ...$args) { $GLOBALS['calls'][] = ['read', $sql]; - return $GLOBALS['ready'] ? ['Field' => 'enabled'] : []; + $GLOBALS['metadataReads']++; + return $GLOBALS['ready'] && ($GLOBALS['mode'] !== 'late' || $GLOBALS['metadataReads'] === 1) ? ['Field' => 'enabled'] : []; } function db_execute($sql, ...$args) { $GLOBALS['calls'][] = ['ddl', $sql]; @@ -55,3 +57,14 @@ function set_default_action() { register_shutdown_function(function () { file_put_contents(getenv('FLOWVIEW_FIXTURE_RESULT'), json_encode(['calls' => $GLOBALS['calls'], 'status' => http_response_code()])); }); +function db_fetch_row_prepared($sql, $params = [], ...$args) { + $GLOBALS['calls'][] = ['row', $sql, $params]; + if (str_contains($sql, 'reports_queued')) { + return ['id' => 51, 'source' => 'flowview', 'source_id' => 7]; + } + return str_contains($sql, 'plugin_flowview_schedules') ? ['id' => 7, 'query_id' => 3] : ['enabled' => '']; +} +function db_execute_prepared($sql, $params = [], ...$args) { + $GLOBALS['calls'][] = ['write', $sql, $params]; + return true; +} From 8f9e8560ea6a2de7ccc80010cacd2fc9ac6b1692 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 19 Oct 2026 14:05:41 +1030 Subject: [PATCH 14/29] fix: isolate filter schema repair in background upgrade helper --- flowview_upgrade.php | 1 + includes/database.php | 14 -------------- includes/filter_upgrade.php | 18 ++++++++++++++++++ phpunit.xml | 1 + tests/README.md | 3 ++- tests/Unit/FilterLifecycleRegressionTest.php | 6 ++++-- tests/bootstrap-unit.php | 3 +++ tests/fixtures/filter-lifecycle.php | 2 +- 8 files changed, 30 insertions(+), 18 deletions(-) create mode 100644 includes/filter_upgrade.php diff --git a/flowview_upgrade.php b/flowview_upgrade.php index 3cbf242..8246da5 100644 --- a/flowview_upgrade.php +++ b/flowview_upgrade.php @@ -28,6 +28,7 @@ require_once('./plugins/flowview/includes/functions.php'); require_once('./plugins/flowview/setup.php'); require_once('./plugins/flowview/includes/database.php'); +require_once('./plugins/flowview/includes/filter_upgrade.php'); flowview_connect(); diff --git a/includes/database.php b/includes/database.php index 5dfe0f4..be112cc 100644 --- a/includes/database.php +++ b/includes/database.php @@ -519,17 +519,3 @@ function flowview_db_get_table_column_types($table, mixed $cnn_id = false) { function flowview_filter_status_ready(): bool { return (bool) flowview_db_fetch_row("SHOW COLUMNS FROM plugin_flowview_queries LIKE 'enabled'", false); } - -/** Repair legacy state only from the background upgrade process. */ -function flowview_filter_status_upgrade(): bool { - if (!flowview_db_table_exists('plugin_flowview_queries')) { - return false; - } - if (flowview_filter_status_ready()) { - return true; - } - if (flowview_db_execute("ALTER TABLE plugin_flowview_queries ADD COLUMN enabled char(2) NOT NULL DEFAULT 'on' AFTER name", false)) { - return true; - } - return flowview_filter_status_ready(); -} diff --git a/includes/filter_upgrade.php b/includes/filter_upgrade.php new file mode 100644 index 0000000..73ed2a6 --- /dev/null +++ b/includes/filter_upgrade.php @@ -0,0 +1,18 @@ +includes/functions.php includes/functions-pre13.php includes/database.php + includes/filter_upgrade.php diff --git a/tests/README.md b/tests/README.md index 35d6c49..a744649 100644 --- a/tests/README.md +++ b/tests/README.md @@ -40,7 +40,8 @@ install/version/uninstall structure without executing plugin source. The filter lifecycle regressions also launch the actual web and upgrade entrypoints in subprocesses with isolated core API fixtures. They verify that all three web -pages defer schema repair, including same-version installations, and that the +pages do not load the schema-mutation helper and defer repair, including +same-version installations, and that the background worker registers before attempting repair. The queue regression uses an in-memory SQLite database to check persisted ordering across multiple passes: a full failed batch remains pending while healthy rows advance on the next pass. diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index 4dba2dd..0328363 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -49,7 +49,8 @@ [$exit, $output] = ($this->runFixture)([PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/' . $page], $ready ? 'ready' : 'pending'); $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); $actions = array_column($result['calls'], 0); - expect($exit)->toBe(0)->and($actions)->not->toContain('ddl'); + expect($exit)->toBe(0)->and($actions)->not->toContain('ddl') + ->and($result['repair_loaded'])->toBeFalse(); if ($ready) { expect($output)->toBe('')->and($actions)->toContain('page-ready')->not->toContain('background'); } else { @@ -64,7 +65,8 @@ $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); expect($result)->toBeArray($output); $actions = array_column($result['calls'], 0); - expect($exit)->toBe($code)->and($actions[0])->toBe('register') + expect($exit)->toBe($code)->and($result['repair_loaded'])->toBeTrue() + ->and($actions[0])->toBe('register') ->and(array_filter($actions, fn ($action) => $action === 'ddl'))->toHaveCount($writes); })->with([['pending', 0, 1], ['failed', 1, 1], ['ready', 0, 0], ['busy', 1, 0]]); it('retains a full failing snapshot while advancing to healthy rows on the next pass', function () { diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 4aefb9c..7039e88 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -518,3 +518,6 @@ function reports_load_format_file($format, &$output, &$report_tag, &$theme) { return true; } } + +// Schema-mutation helpers are loaded explicitly for isolated upgrade tests. +require_once dirname(__DIR__) . '/includes/filter_upgrade.php'; diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php index 2abca21..969dd1e 100644 --- a/tests/fixtures/filter-lifecycle.php +++ b/tests/fixtures/filter-lifecycle.php @@ -55,7 +55,7 @@ function set_default_action() { exit; } register_shutdown_function(function () { - file_put_contents(getenv('FLOWVIEW_FIXTURE_RESULT'), json_encode(['calls' => $GLOBALS['calls'], 'status' => http_response_code()])); + file_put_contents(getenv('FLOWVIEW_FIXTURE_RESULT'), json_encode(['calls' => $GLOBALS['calls'], 'status' => http_response_code(), 'repair_loaded' => function_exists('flowview_filter_status_upgrade')])); }); function db_fetch_row_prepared($sql, $params = [], ...$args) { $GLOBALS['calls'][] = ['row', $sql, $params]; From fbee2fd30eaff039f58c8eab1743321616b467a8 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:33:43 +1030 Subject: [PATCH 15/29] fix: distinguish empty scheduled reports from query failures --- includes/functions.php | 26 +++++++++++++------------- run_schedule.php | 4 ++-- tests/Unit/FilterStatusTest.php | 22 +++++++++++++++++----- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/includes/functions.php b/includes/functions.php index c074b5b..721fafd 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -1916,7 +1916,7 @@ function get_port_name($port_num, int $port_proto = 6) { * @param mixed $id * @param mixed $report_id * - * @return bool False when schema readiness requires retry; true otherwise. + * @return bool False when schema readiness or query execution requires retry; true otherwise. */ function plugin_flowview_run_schedule($id, $report_id): bool { global $config; @@ -1960,20 +1960,24 @@ function plugin_flowview_run_schedule($id, $report_id): bool { /* get the data and append to the boby */ $data = load_data_for_filter($schedule['query_id'], $start, $end); - if ($data !== false) { - $body .= $data['table']; + if ($data === false || ($data['data'] ?? null) === false) { + cacti_log('FlowView scheduled report query failed; retaining the occurrence for retry.', false, 'FLOWVIEW'); + return false; + } + if (empty($data['data'])) { + cacti_log(sprintf('WARNING: Running of Flowview Schedule %s Returned not Data. Please Check your Flowview Schedule', $report_id), false, 'FLOWVIEW'); + return true; } + $body .= $data['table']; $body_text = html_escape($schedule['title']) . PHP_EOL . PHP_EOL; $body_text .= 'From ' . date('Y-m-d H:i:s', $start) . ' to ' . date('Y-m-d H:i:s', $end) . PHP_EOL; $body_text .= 'Using Query \'' . html_escape($query['name']) . '\'' . PHP_EOL . PHP_EOL; - if ($data !== false) { - $body_text .= implode(', ', array_keys($data['data'][0])) . PHP_EOL; + $body_text .= implode(', ', array_keys($data['data'][0])) . PHP_EOL; - foreach($data['data'] as $row) { - $body_text .= implode(', ', array_values($row)) . PHP_EOL; - } + foreach($data['data'] as $row) { + $body_text .= implode(', ', array_values($row)) . PHP_EOL; } /* process the format file */ @@ -2001,11 +2005,7 @@ function plugin_flowview_run_schedule($id, $report_id): bool { $headers['User-Agent'] = 'Cacti-FlowView-v' . $version; $headers['X-Priority'] = '1'; - if (cacti_sizeof($data['data'])) { - reports_log_and_notify($report_id, $start_time, 'html', 'flowview', $id, $subject, $data['data'], $body, $body_html, $body_text, [], $headers); - } else { - cacti_log(sprintf('WARNING: Running of Flowview Schedule %s Returned not Data. Please Check your Flowview Schedule', $report_id), false, 'FLOWVIEW'); - } + reports_log_and_notify($report_id, $start_time, 'html', 'flowview', $id, $subject, $data['data'], $body, $body_html, $body_text, [], $headers); return true; } diff --git a/run_schedule.php b/run_schedule.php index c13f28c..b683c09 100644 --- a/run_schedule.php +++ b/run_schedule.php @@ -162,8 +162,8 @@ if (cacti_sizeof($schedule)) { if (!plugin_flowview_run_schedule($id, $report_id)) { - // A later readiness failure must preserve the original occurrence - // and its schedule timestamps just like the initial check does. + // A deferred or failed preparation must preserve the occurrence + // and schedule timestamps just like the initial readiness check. if (!$force) { unregister_process('flowsched', "child_$id", 0); } diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index 8e5e3e3..a2fa0ba 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -87,7 +87,7 @@ expect($span)->toBe(['begin_now' => 200, 'end_now' => 2000]); }); -it('returns success after preparing an enabled schedule', function () { +it('distinguishes prepared records, an empty period and a failed query', function ($data, $success) { plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 3, 'title' => 'Fixture', 'format_file' => 'default']); plugin_test_queue_db_result('db_fetch_row_prepared', ['enabled' => 'on', 'timespan' => 1, 'name' => 'Fixture']); @@ -97,7 +97,19 @@ set_request_var('date1', date('Y-m-d H:i:s', 100)); set_request_var('date2', date('Y-m-d H:i:s', 2000)); $key = get_flowview_session_key(3, 100, 2000); - $_SESSION['sess_flowdata'][$key] = ['timeout' => time() + 60, 'data' => ['table' => 'Fixture', 'data' => [['bytes' => 42]]]]; - expect(plugin_flowview_run_schedule(7, 51))->toBeTrue(); - expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->toContain('reports_load_format_file'); -}); + $_SESSION['sess_flowdata'][$key] = ['timeout' => time() + 60, 'data' => $data]; + $GLOBALS['__test_cacti_log'] = []; + expect(plugin_flowview_run_schedule(7, 51))->toBe($success); + $formatted = in_array('reports_load_format_file', array_column($GLOBALS['__test_db_calls'], 'fn'), true); + expect($formatted)->toBe(!empty($data['data'])); + if ($data === false || ($data['data'] ?? null) === false) { + expect(implode(' ', $GLOBALS['__test_cacti_log']))->toContain('retry'); + } elseif (empty($data['data'])) { + expect(implode(' ', $GLOBALS['__test_cacti_log']))->toContain('Returned not Data'); + } +})->with([ + [['table' => 'Fixture', 'data' => [['bytes' => 42]]], true], + [['table' => 'Fixture', 'data' => []], true], + [false, false], + [['table' => 'Fixture', 'data' => false], false], +]); From 5eefb7dc221298f0bdb1828c6448e11945e79caa Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:30:49 +1030 Subject: [PATCH 16/29] fix: distinguish failed lookups and uncached empty query results --- includes/functions.php | 22 ++++++++++ run_schedule.php | 11 +++++ tests/Unit/FilterLifecycleRegressionTest.php | 16 ++++++++ tests/Unit/FilterStatusTest.php | 43 ++++++++++++++++++++ tests/bootstrap-unit.php | 11 +++++ tests/fixtures/filter-lifecycle.php | 7 +++- 6 files changed, 109 insertions(+), 1 deletion(-) diff --git a/includes/functions.php b/includes/functions.php index 721fafd..8e6cf72 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -1932,11 +1932,22 @@ function plugin_flowview_run_schedule($id, $report_id): bool { WHERE id = ?', [$id]); + if ($schedule === false) { + return false; + } + if (!$schedule) { + return true; + } + $query = flowview_db_fetch_row_prepared('SELECT * FROM plugin_flowview_queries WHERE id = ?', [$schedule['query_id']]); + if ($query === false) { + return false; + } + if (empty($query) || ($query['enabled'] ?? '') !== 'on') { return true; } @@ -2763,6 +2774,10 @@ function run_flow_query($session, $query_id, $start, $end) { WHERE id = ?', [$query_id]); + if (!$data) { + return false; + } + $title = flowview_db_fetch_cell_prepared('SELECT name FROM plugin_flowview_queries WHERE id = ?', @@ -3571,6 +3586,13 @@ function run_flow_query($session, $query_id, $start, $end) { } } + if ($results === false) { + return false; + } + if (!$results) { + return ['data' => [], 'table' => '', 'title' => $title]; + } + $output = $data; $output['data'] = $results; diff --git a/run_schedule.php b/run_schedule.php index b683c09..51b6cc1 100644 --- a/run_schedule.php +++ b/run_schedule.php @@ -136,6 +136,10 @@ AND rq.id = ?', ['flowview', $report_id]); + if ($report === false) { + exit(75); + } + if (cacti_sizeof($report)) { $id = $report['source_id']; @@ -160,6 +164,13 @@ WHERE id = ?', [$id]); + if ($schedule === false) { + if (!$force) { + unregister_process('flowsched', "child_$id", 0); + } + exit(75); + } + if (cacti_sizeof($schedule)) { if (!plugin_flowview_run_schedule($id, $report_id)) { // A deferred or failed preparation must preserve the occurrence diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index 0328363..b7e679a 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -111,3 +111,19 @@ $config['base_path'] = $oldBase; } })->with([false, true])->with([false, true]); + +it('returns retry from the real child when a queue, schedule or filter lookup fails', function ($mode, $native) { + $command = [PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/run_schedule.php', '--report-id=' . ($native ? 7 : 51)]; + if ($native) { + $command[] = '--queue-id=51'; + } + [$exit] = ($this->runFixture)($command, $mode); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + expect($exit)->toBe(75); + $actions = array_column($result['calls'], 0); + expect($actions)->not->toContain('write'); + if ($mode !== 'report-read-failure') { + $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); + expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0]]]); + } +})->with(['report-read-failure', 'schedule-read-failure', 'filter-read-failure'])->with([false, true]); diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index a2fa0ba..20531c7 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -113,3 +113,46 @@ [false, false], [['table' => 'Fixture', 'data' => false], false], ]); + +it('retries failed filter lookups while consuming genuinely absent or disabled filters', function ($filter, $success) { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 3]); + plugin_test_queue_db_result('db_fetch_row_prepared', $filter); + expect(plugin_flowview_run_schedule(7, 51))->toBe($success); +})->with([[false, false], [[], true], [['enabled' => ''], true]]); + +it('distinguishes uncached empty query execution from failure in scheduled preparation', function ($rows, $success) { + $start = strtotime('2024-01-01 00:00:00 UTC'); + $end = $start + 3600; + $query = ['id' => 3, 'name' => 'Fixture', 'enabled' => 'on', 'timespan' => 1, 'statistics' => 9, 'printed' => 0, 'sortfield' => 'bytes', 'cutofflines' => 100]; + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row', ['min_date' => date('Y-m-d H:i:s', $start), 'max_date' => date('Y-m-d H:i:s', $end), 'table_partition' => '']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 3, 'title' => 'Fixture', 'format_file' => 'default']); + plugin_test_queue_db_result('db_fetch_row_prepared', $query); + plugin_test_queue_db_result('db_fetch_row_prepared', $query); + plugin_test_queue_db_result('get_timespan', ['begin_now' => $start, 'end_now' => $end]); + // arrays.php reads three choice lists each time; no result-session cache is seeded. + for ($i = 0; $i < 6; $i++) { + plugin_test_queue_db_result('db_fetch_assoc', []); + } + plugin_test_queue_db_result('db_fetch_assoc', [['table' => 'plugin_flowview_raw_202400000']]); + plugin_test_queue_db_result('db_fetch_assoc_prepared', $rows); + $GLOBALS['__test_config']['flowview_partition'] = 1; + $GLOBALS['__test_config']['flowview_parallel_threads'] = 1; + expect(plugin_flowview_run_schedule(7, 51))->toBe($success); + $queries = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_fetch_assoc_prepared')); + expect($queries)->toHaveCount(1)->and($queries[0]['sql'])->toContain('FROM plugin_flowview_raw_202400000'); + expect($_SESSION)->not->toHaveKey('sess_flowdata'); +})->with([[[], true], [false, false]]); + +it('distinguishes failed and absent schedule lookups before reading a filter', function ($schedule, $success) { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', $schedule); + expect(plugin_flowview_run_schedule(7, 51))->toBe($success); + $reads = array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_fetch_row_prepared'); + expect($reads)->toHaveCount(1); +})->with([[false, false], [[], true]]); +it('fails an uncached query cleanly when its own filter lookup fails or is absent', function ($filter) { + plugin_test_queue_db_result('db_fetch_row_prepared', $filter); + expect(run_flow_query(false, 3, 1704067200, 1704070800))->toBeFalse(); +})->with([[false], [[]]]); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 7039e88..52af616 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -521,3 +521,14 @@ function reports_load_format_file($format, &$output, &$report_tag, &$theme) { // Schema-mutation helpers are loaded explicitly for isolated upgrade tests. require_once dirname(__DIR__) . '/includes/filter_upgrade.php'; + +if (!function_exists('cacti_session_close')) { + function cacti_session_close() { + $GLOBALS['__test_db_calls'][] = ['fn' => 'cacti_session_close', 'sql' => '', 'params' => []]; + } +} +if (!function_exists('array_rekey')) { + function array_rekey($rows, $key, $value) { + return array_column($rows, $value, $key); + } +} diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php index 969dd1e..7cc2373 100644 --- a/tests/fixtures/filter-lifecycle.php +++ b/tests/fixtures/filter-lifecycle.php @@ -4,7 +4,7 @@ $config = ['base_path' => dirname(__DIR__), 'poller_id' => 1]; $local_db_cnn_id = 'fixture'; $mode = getenv('FLOWVIEW_FIXTURE_MODE'); -$ready = in_array($mode, ['ready', 'late', 'child-disabled'], true); +$ready = in_array($mode, ['ready', 'late', 'child-disabled', 'report-read-failure', 'schedule-read-failure', 'filter-read-failure'], true); $metadataReads = 0; $calls = []; const MESSAGE_LEVEL_INFO = 1; @@ -59,6 +59,11 @@ function set_default_action() { }); function db_fetch_row_prepared($sql, $params = [], ...$args) { $GLOBALS['calls'][] = ['row', $sql, $params]; + if (($GLOBALS['mode'] === 'report-read-failure' && str_contains($sql, 'reports_queued')) + || ($GLOBALS['mode'] === 'schedule-read-failure' && str_contains($sql, 'plugin_flowview_schedules')) + || ($GLOBALS['mode'] === 'filter-read-failure' && str_contains($sql, 'plugin_flowview_queries'))) { + return false; + } if (str_contains($sql, 'reports_queued')) { return ['id' => 51, 'source' => 'flowview', 'source_id' => 7]; } From 7e4519047771be333e73896abb6629df200311c8 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:48:13 +1030 Subject: [PATCH 17/29] fix: propagate partition metadata failures to scheduled retries --- includes/functions.php | 30 ++++++++++++++++++++++-------- tests/Unit/FilterStatusTest.php | 20 +++++++++++++++----- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/includes/functions.php b/includes/functions.php index 8e6cf72..22cfab7 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -2339,7 +2339,7 @@ function get_date_filter($sql_range, &$sql_range_params, $start, $end, int $rang * @param mixed $start * @param mixed $end * - * @return mixed + * @return array|false Matching partitions, or false when metadata discovery fails. */ function get_tables_for_query($start, $end = null) { global $config, $graph_timespans; @@ -2365,6 +2365,10 @@ function get_tables_for_query($start, $end = null) { FROM information_schema.TABLES WHERE TABLE_NAME LIKE "plugin_flowview_raw_%"'); + if ($tables === false) { + return false; + } + if (cacti_sizeof($tables)) { foreach($tables as $t) { $parts = explode('_', $t['table']); @@ -2376,20 +2380,27 @@ function get_tables_for_query($start, $end = null) { } if ($partition >= $start_part && $partition <= $end_part) { - $inc_tables[$t['table']]['table_name'] = $t['table']; - if (!isset($_SESSION['sess_flowview_table_details'][$t['table']])) { $details = flowview_db_fetch_row("SELECT MIN(start_time) AS min_date, MAX(end_time) AS max_date, '' AS table_partition FROM {$t['table']}"); - if (!cacti_sizeof($details)) { - cacti_log("WARNING: No Details for {$t['table']} returned"); - } - - $_SESSION['sess_flowview_table_details'][$t['table']] = $details; } else { $details = $_SESSION['sess_flowview_table_details'][$t['table']]; } + if (!$details) { + unset($_SESSION['sess_flowview_table_details'][$t['table']]); + cacti_log("WARNING: No Details for {$t['table']} returned"); + return false; + } + // MIN/MAX return NULL for an empty partition. Recheck it on the + // next request rather than caching empty bounds while it grows. + if ($details['min_date'] === null && $details['max_date'] === null) { + unset($_SESSION['sess_flowview_table_details'][$t['table']]); + continue; + } + $_SESSION['sess_flowview_table_details'][$t['table']] = $details; + $inc_tables[$t['table']]['table_name'] = $t['table']; + $inc_tables[$t['table']]['table_partition'] = $details['table_partition']; $inc_tables[$t['table']]['min_date'] = $details['min_date']; $inc_tables[$t['table']]['max_date'] = $details['max_date']; @@ -3435,6 +3446,9 @@ function run_flow_query($session, $query_id, $start, $end) { } $tables = get_tables_for_query($start, $end); + if ($tables === false) { + return false; + } $sql = ''; $all_params = []; $results = []; diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index 20531c7..3606af5 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -121,12 +121,12 @@ expect(plugin_flowview_run_schedule(7, 51))->toBe($success); })->with([[false, false], [[], true], [['enabled' => ''], true]]); -it('distinguishes uncached empty query execution from failure in scheduled preparation', function ($rows, $success) { +it('distinguishes uncached empty query execution from failure in scheduled preparation', function ($rows, $success, $catalog = 'normal', $details = 'normal') { $start = strtotime('2024-01-01 00:00:00 UTC'); $end = $start + 3600; $query = ['id' => 3, 'name' => 'Fixture', 'enabled' => 'on', 'timespan' => 1, 'statistics' => 9, 'printed' => 0, 'sortfield' => 'bytes', 'cutofflines' => 100]; plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); - plugin_test_queue_db_result('db_fetch_row', ['min_date' => date('Y-m-d H:i:s', $start), 'max_date' => date('Y-m-d H:i:s', $end), 'table_partition' => '']); + plugin_test_queue_db_result('db_fetch_row', $details === 'normal' ? ['min_date' => date('Y-m-d H:i:s', $start), 'max_date' => date('Y-m-d H:i:s', $end), 'table_partition' => ''] : $details); plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 3, 'title' => 'Fixture', 'format_file' => 'default']); plugin_test_queue_db_result('db_fetch_row_prepared', $query); plugin_test_queue_db_result('db_fetch_row_prepared', $query); @@ -135,15 +135,25 @@ for ($i = 0; $i < 6; $i++) { plugin_test_queue_db_result('db_fetch_assoc', []); } - plugin_test_queue_db_result('db_fetch_assoc', [['table' => 'plugin_flowview_raw_202400000']]); + plugin_test_queue_db_result('db_fetch_assoc', $catalog === 'normal' ? [['table' => 'plugin_flowview_raw_202400000']] : $catalog); plugin_test_queue_db_result('db_fetch_assoc_prepared', $rows); $GLOBALS['__test_config']['flowview_partition'] = 1; $GLOBALS['__test_config']['flowview_parallel_threads'] = 1; expect(plugin_flowview_run_schedule(7, 51))->toBe($success); $queries = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_fetch_assoc_prepared')); - expect($queries)->toHaveCount(1)->and($queries[0]['sql'])->toContain('FROM plugin_flowview_raw_202400000'); + if ($catalog === 'normal' && $details === 'normal') { + expect($queries)->toHaveCount(1)->and($queries[0]['sql'])->toContain('FROM plugin_flowview_raw_202400000'); + } else { + expect($queries)->toHaveCount(0); + expect($_SESSION['sess_flowview_table_details'] ?? [])->not->toHaveKey('plugin_flowview_raw_202400000'); + } expect($_SESSION)->not->toHaveKey('sess_flowdata'); -})->with([[[], true], [false, false]]); +})->with([ + [[], true], [false, false], + [[], false, false], [[], true, []], + [[], false, 'normal', false], [[], false, 'normal', []], + [[], true, 'normal', ['min_date' => null, 'max_date' => null, 'table_partition' => '']], +]); it('distinguishes failed and absent schedule lookups before reading a filter', function ($schedule, $success) { plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); From c66e32c2fdec6d29153673eb46e4e1dd9a34db69 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 19 Oct 2026 20:04:07 +1030 Subject: [PATCH 18/29] fix: claim scheduled work atomically and reject invalid parallel results --- includes/functions.php | 4 ++-- tests/Unit/FilterLifecycleRegressionTest.php | 1 + tests/Unit/FilterQueueBatchTest.php | 12 +++++++++++ tests/Unit/FilterStatusTest.php | 21 +++++++++++++++++--- tests/bootstrap-unit.php | 6 ++++++ tests/fixtures/filter-queue.php | 12 +++++++++-- 6 files changed, 49 insertions(+), 7 deletions(-) diff --git a/includes/functions.php b/includes/functions.php index 22cfab7..32b144d 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -3600,7 +3600,7 @@ function run_flow_query($session, $query_id, $start, $end) { } } - if ($results === false) { + if (!is_array($results)) { return false; } if (!$results) { @@ -7432,7 +7432,7 @@ function flowview_reports_run(int $id): bool { return false; } require_once $config['base_path'] . '/lib/poller.php'; - if (!db_execute_prepared('UPDATE reports_queued SET status=?, start_time=? WHERE id=? AND source=?', ['running', date('Y-m-d H:i:s'), $id, 'flowview'])) { + if (!db_execute_prepared('UPDATE reports_queued SET status=?, start_time=? WHERE id=? AND source=? AND status=?', ['running', date('Y-m-d H:i:s'), $id, 'flowview', 'pending']) || db_affected_rows() !== 1) { return false; } $output = []; diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index b7e679a..ea98b93 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -77,6 +77,7 @@ $result = json_decode($output, true); expect($exit)->toBe(0)->and($result['first'])->toBe(0)->and($result['second'])->toBe(2)->and($result['third'])->toBe(0) ->and($result['attempts'])->toBe([1, 2, 3, 4, 1, 2]) + ->and($result['lostClaim'])->toBeFalse()->and($result['claimedStatus'])->toBe(['status' => 'running']) ->and($result['afterFirst'])->toHaveCount(4) ->and($result['remaining'])->toBe([['id' => 1, 'status' => 'pending'], ['id' => 2, 'status' => 'pending']]); }); diff --git a/tests/Unit/FilterQueueBatchTest.php b/tests/Unit/FilterQueueBatchTest.php index 55187b8..17af5a3 100644 --- a/tests/Unit/FilterQueueBatchTest.php +++ b/tests/Unit/FilterQueueBatchTest.php @@ -124,3 +124,15 @@ $deletes = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM processes'))); expect($deletes)->toHaveCount(1)->and($deletes[0]['params'])->toBe(['flowsched', 'child_7', 0, 2147483647]); }); + +it('does not launch or reset an occurrence when another dispatcher already owns its claim', function () { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); + plugin_test_queue_db_result('db_affected_rows', 0); + expect(flowview_reports_run(51))->toBeFalse(); + $calls = $GLOBALS['__test_db_calls']; + expect(array_column($calls, 'fn'))->not->toContain('exec_with_timeout'); + $writes = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'db_execute_prepared')); + expect($writes)->toHaveCount(1)->and($writes[0]['sql'])->toContain('AND status=?') + ->and($writes[0]['params'][4])->toBe('pending'); +}); diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index 3606af5..609ab04 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -121,7 +121,7 @@ expect(plugin_flowview_run_schedule(7, 51))->toBe($success); })->with([[false, false], [[], true], [['enabled' => ''], true]]); -it('distinguishes uncached empty query execution from failure in scheduled preparation', function ($rows, $success, $catalog = 'normal', $details = 'normal') { +it('distinguishes uncached empty query execution from failure in scheduled preparation', function ($rows, $success, $catalog = 'normal', $details = 'normal', $threads = 1, $parallelResult = '[]') { $start = strtotime('2024-01-01 00:00:00 UTC'); $end = $start + 3600; $query = ['id' => 3, 'name' => 'Fixture', 'enabled' => 'on', 'timespan' => 1, 'statistics' => 9, 'printed' => 0, 'sortfield' => 'bytes', 'cutofflines' => 100]; @@ -138,10 +138,21 @@ plugin_test_queue_db_result('db_fetch_assoc', $catalog === 'normal' ? [['table' => 'plugin_flowview_raw_202400000']] : $catalog); plugin_test_queue_db_result('db_fetch_assoc_prepared', $rows); $GLOBALS['__test_config']['flowview_partition'] = 1; - $GLOBALS['__test_config']['flowview_parallel_threads'] = 1; + $GLOBALS['__test_config']['flowview_parallel_threads'] = $threads; + if ($threads > 1) { + $GLOBALS['debug'] = false; + $GLOBALS['__test_config']['flowview_parallel_runlimit'] = 5; + foreach (['Fixture', 71, 0, $parallelResult, ''] as $value) { + plugin_test_queue_db_result('db_fetch_cell_prepared', $value); + } + plugin_test_queue_db_result('db_fetch_cell', 0); + } expect(plugin_flowview_run_schedule(7, 51))->toBe($success); $queries = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_fetch_assoc_prepared')); - if ($catalog === 'normal' && $details === 'normal') { + if ($threads > 1) { + $reads = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_fetch_cell_prepared' && str_contains($c['sql'], 'SELECT results'))); + expect($reads)->toHaveCount(1)->and($reads[0]['params'])->toBe([71]); + } elseif ($catalog === 'normal' && $details === 'normal') { expect($queries)->toHaveCount(1)->and($queries[0]['sql'])->toContain('FROM plugin_flowview_raw_202400000'); } else { expect($queries)->toHaveCount(0); @@ -153,6 +164,10 @@ [[], false, false], [[], true, []], [[], false, 'normal', false], [[], false, 'normal', []], [[], true, 'normal', ['min_date' => null, 'max_date' => null, 'table_partition' => '']], + [[], true, 'normal', 'normal', 2, '[]'], + [[], false, 'normal', 'normal', 2, false], + [[], false, 'normal', 'normal', 2, 'null'], + [[], false, 'normal', 'normal', 2, 'invalid-json'], ]); it('distinguishes failed and absent schedule lookups before reading a filter', function ($schedule, $success) { diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 52af616..6a5e2a7 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -532,3 +532,9 @@ function array_rekey($rows, $key, $value) { return array_column($rows, $value, $key); } } + +if (!function_exists('db_debug')) { + function db_debug($message) { + $GLOBALS['__test_db_calls'][] = ['fn' => 'db_debug', 'sql' => '', 'params' => [$message]]; + } +} diff --git a/tests/fixtures/filter-queue.php b/tests/fixtures/filter-queue.php index f5715da..69ff3cc 100644 --- a/tests/fixtures/filter-queue.php +++ b/tests/fixtures/filter-queue.php @@ -25,7 +25,12 @@ function db_fetch_row_prepared($sql, $params = [], ...$args) { } function db_execute_prepared($sql, $params = [], ...$args) { $stmt = $GLOBALS['db']->prepare($sql); - return $stmt->execute($params); + $result = $stmt->execute($params); + $GLOBALS['affected'] = $stmt->rowCount(); + return $result; +} +function db_affected_rows(...$args) { + return $GLOBALS['affected'] ?? 0; } function exec_with_timeout($command, &$output, &$status, $timeout) { preg_match('/--queue-id=(\d+)/', $command, $match); @@ -46,4 +51,7 @@ function cacti_log(...$args) { $second = flowview_reports_run_pending(1, 2); $remaining = db_fetch_assoc_prepared('SELECT id,status FROM reports_queued ORDER BY id'); $third = flowview_reports_run_pending(1, 2); -echo json_encode(compact('first', 'second', 'third', 'attempts', 'afterFirst', 'remaining')); +$db->exec("INSERT INTO reports_queued VALUES (5, 'flowview', 5, 'running', '2026-01-01 00:00:00', '2026-01-01 00:00:00', 'fixture', 1)"); +$lostClaim = flowview_reports_run(5); +$claimedStatus = db_fetch_row_prepared('SELECT status FROM reports_queued WHERE id=5'); +echo json_encode(compact('first', 'second', 'third', 'attempts', 'afterFirst', 'remaining', 'lostClaim', 'claimedStatus')); From 94d2bc68ea918b67c96a1dbad52abdd54183c042 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 19 Oct 2026 23:19:57 +1030 Subject: [PATCH 19/29] Preserve scheduled retries on parallel query failures --- includes/functions.php | 21 +++++---- tests/Unit/FilterLifecycleRegressionTest.php | 29 ++++++++++++ tests/Unit/FilterStatusTest.php | 28 ++++++++++++ tests/bootstrap-unit.php | 15 ++++++- tests/fixtures/filter-lifecycle.php | 47 +++++++++++++++++--- 5 files changed, 123 insertions(+), 17 deletions(-) diff --git a/includes/functions.php b/includes/functions.php index 32b144d..ed0fcce 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -4269,7 +4269,7 @@ function parallel_database_query_run($requests) { parallel_database_query_cleanup($request_id); } - exit; + return false; } if (cacti_sizeof($requests) == 1) { @@ -4283,14 +4283,15 @@ function parallel_database_query_run($requests) { WHERE id = ?', [$requests[0]]); - $tables = array_rekey( - flowview_db_fetch_assoc('SELECT map_table, map_create, results - FROM parallel_database_query - WHERE id IN(' . implode(', ', $requests) . ')'), - 'map_table', ['map_create', 'results'] - ); + $query = json_decode($reduce_query ?: '', true); + $tables = flowview_db_fetch_assoc('SELECT map_table, map_create, results + FROM parallel_database_query + WHERE id IN(' . implode(', ', $requests) . ')'); - if ($reduce_query != '' && cacti_sizeof($tables)) { + // Missing reduction metadata is a failed report, not a successful empty result. + if (is_array($query) && isset($query['sql_query'], $query['sql_where'], $query['sql_groupby'], $query['sql_having'], $query['sql_order'], $query['sql_limit'], $query['sql_params']) + && is_array($query['sql_params']) && is_array($tables) && count($tables) === count($requests)) { + $tables = array_rekey($tables, 'map_table', ['map_create', 'results']); $i = 0; $temp_table = ''; @@ -4340,8 +4341,6 @@ function parallel_database_query_run($requests) { $i++; } - $query = json_decode($reduce_query, true); - $final_query = "{$query['sql_query']} FROM $temp_table {$query['sql_where']} @@ -4358,7 +4357,7 @@ function parallel_database_query_run($requests) { flowview_db_execute("DROP TEMPORARY TABLE IF EXISTS $temp_table"); } else { - $results = []; + $results = false; } } diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index ea98b93..f3aa7c6 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -128,3 +128,32 @@ expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0]]]); } })->with(['report-read-failure', 'schedule-read-failure', 'filter-read-failure'])->with([false, true]); + +it('retains a timed out parallel report through the actual child and parent', function ($native) { + $command = [PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/run_schedule.php', '--report-id=' . ($native ? 7 : 51)]; + if ($native) { + $command[] = '--queue-id=51'; + } + [$exit, $output] = ($this->runFixture)($command, 'parallel-timeout'); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + expect($exit)->toBe(75)->and($output)->toBe(''); + $writes = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'write')); + expect(array_filter($writes, fn ($c) => str_contains($c[1], 'UPDATE plugin_flowview_schedules')))->toBe([]); + $cleanup = array_values(array_filter($writes, fn ($c) => str_starts_with($c[1], 'DELETE FROM parallel_database_query_shard'))); + expect(array_column($cleanup, 2))->toBe([[71]]); + $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); + expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0]]]); + global $config; + $oldBase = $config['base_path']; + $config['base_path'] = $this->fixtureRoot; + try { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); + plugin_test_queue_db_result('exec_with_timeout', $exit); + expect(flowview_reports_run(51))->toBeFalse(); + $deletes = array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM reports_queued')); + expect($deletes)->toBe([]); + } finally { + $config['base_path'] = $oldBase; + } +})->with([false, true]); diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index 609ab04..24e4767 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -181,3 +181,31 @@ plugin_test_queue_db_result('db_fetch_row_prepared', $filter); expect(run_flow_query(false, 3, 1704067200, 1704070800))->toBeFalse(); })->with([[false], [[]]]); + +it('preserves two-request reduce lookup failures and cleans up both requests', function ($reduce, $success, $metadata = 'normal') { + $GLOBALS['debug'] = false; + $GLOBALS['__test_config']['flowview_parallel_runlimit'] = 5; + $query = ['sql_query' => 'SELECT SUM(bytes) bytes', 'sql_where' => '', 'sql_groupby' => '', 'sql_having' => '', 'sql_order' => '', 'sql_limit' => '', 'sql_params' => []]; + foreach ([0, 0, $reduce === 'valid' ? json_encode($query) : $reduce, '', ''] as $value) { + plugin_test_queue_db_result('db_fetch_cell_prepared', $value); + } + plugin_test_queue_db_result('db_fetch_cell', 0); + plugin_test_queue_db_result('db_fetch_assoc', $metadata === 'normal' ? [ + ['map_table' => 'fixture_map_71', 'map_create' => 'CREATE TABLE fixture_map_71 (bytes bigint)', 'results' => '[]'], + ['map_table' => 'fixture_map_72', 'map_create' => 'CREATE TABLE fixture_map_72 (bytes bigint)', 'results' => '[]'], + ] : $metadata); + expect(parallel_database_query_run([71, 72]))->toBe($success ? [] : false); + $cleanup = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM parallel_database_query_shard'))); + expect(array_column($cleanup, 'params'))->toBe([[71], [72]]); +})->with([[false, false], ['', false], ['null', false], ['bad-json', false], ['valid', true], ['valid', false, false], ['valid', false, []], ['{}', false]]); + +it('returns a failed result after cleaning up timed out parallel requests', function () { + $GLOBALS['debug'] = false; + $GLOBALS['__test_config']['flowview_parallel_runlimit'] = 0; + foreach ([0, 0, '', ''] as $value) { + plugin_test_queue_db_result('db_fetch_cell_prepared', $value); + } + expect(parallel_database_query_run([71, 72]))->toBeFalse(); + $cleanup = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM parallel_database_query_shard'))); + expect(array_column($cleanup, 'params'))->toBe([[71], [72]]); +}); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 6a5e2a7..7af683e 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -305,6 +305,12 @@ function is_realm_allowed($realm) { } } +if (!function_exists('raise_message_javascript')) { + function raise_message_javascript(...$args) { + raise_message(...$args); + } +} + if (!function_exists('raise_message')) { function raise_message($id, $text = '', $level = 0) { $GLOBALS['__test_messages'][] = array($id, $text, $level); @@ -529,7 +535,14 @@ function cacti_session_close() { } if (!function_exists('array_rekey')) { function array_rekey($rows, $key, $value) { - return array_column($rows, $value, $key); + if (!is_array($value)) { + return array_column($rows, $value, $key); + } + $result = []; + foreach ($rows as $row) { + $result[$row[$key]] = array_intersect_key($row, array_flip($value)); + } + return $result; } } diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php index 7cc2373..6d97505 100644 --- a/tests/fixtures/filter-lifecycle.php +++ b/tests/fixtures/filter-lifecycle.php @@ -4,10 +4,11 @@ $config = ['base_path' => dirname(__DIR__), 'poller_id' => 1]; $local_db_cnn_id = 'fixture'; $mode = getenv('FLOWVIEW_FIXTURE_MODE'); -$ready = in_array($mode, ['ready', 'late', 'child-disabled', 'report-read-failure', 'schedule-read-failure', 'filter-read-failure'], true); +$ready = in_array($mode, ['ready', 'late', 'child-disabled', 'report-read-failure', 'schedule-read-failure', 'filter-read-failure', 'parallel-timeout'], true); $metadataReads = 0; $calls = []; const MESSAGE_LEVEL_INFO = 1; +const MESSAGE_LEVEL_ERROR = 2; function __(string $text, ...$args) { return $text; } @@ -18,20 +19,29 @@ function cacti_sizeof($value) { return is_array($value) ? count($value) : 0; } function read_config_option($name) { - return $name === 'path_php_binary' ? PHP_BINARY : ''; + return ['path_php_binary' => PHP_BINARY, 'flowview_parallel_threads' => 2, 'flowview_partition' => 1, 'flowview_parallel_time_to_live' => 21600, 'flowview_parallel_runlimit' => 0][$name] ?? ''; } function cacti_log(...$args) { } function raise_message(...$args) { } -function db_fetch_cell_prepared(...$args) { - return 'fixture-version'; +function db_fetch_cell_prepared($sql, ...$args) { + if (str_contains($sql, 'SELECT name')) { + return 'Fixture'; + } + if (str_contains($sql, 'SELECT id')) { + return 71; + } + return str_contains($sql, 'parallel_database_query') ? '' : 'fixture-version'; } function db_fetch_cell(...$args) { return str_contains($args[0], 'SELECT version') ? 'fixture-version' : 1; } function db_fetch_row($sql, ...$args) { $GLOBALS['calls'][] = ['read', $sql]; + if (str_contains($sql, 'MIN(start_time)')) { + return ['min_date' => '2024-01-01 00:00:00', 'max_date' => '2024-01-01 01:00:00', 'table_partition' => '']; + } $GLOBALS['metadataReads']++; return $GLOBALS['ready'] && ($GLOBALS['mode'] !== 'late' || $GLOBALS['metadataReads'] === 1) ? ['Field' => 'enabled'] : []; } @@ -67,9 +77,36 @@ function db_fetch_row_prepared($sql, $params = [], ...$args) { if (str_contains($sql, 'reports_queued')) { return ['id' => 51, 'source' => 'flowview', 'source_id' => 7]; } - return str_contains($sql, 'plugin_flowview_schedules') ? ['id' => 7, 'query_id' => 3] : ['enabled' => '']; + if (str_contains($sql, 'plugin_flowview_schedules')) { + return ['id' => 7, 'query_id' => 3, 'title' => 'Fixture', 'format_file' => 'default']; + } + return $GLOBALS['mode'] === 'parallel-timeout' + ? ['id' => 3, 'name' => 'Fixture', 'enabled' => 'on', 'timespan' => 1, 'statistics' => 9, 'printed' => 0, 'sortfield' => 'bytes', 'cutofflines' => 100] + : ['enabled' => '']; } function db_execute_prepared($sql, $params = [], ...$args) { $GLOBALS['calls'][] = ['write', $sql, $params]; return true; } + +function db_fetch_assoc($sql, ...$args) { + return str_contains($sql, 'information_schema') ? [['table' => 'plugin_flowview_raw_202400000']] : []; +} +function db_fetch_assoc_prepared(...$args) { + return []; +} +function get_timespan(&$span, ...$args) { + $span = ['begin_now' => 1704067200, 'end_now' => 1704070800]; +} +function read_user_setting(...$args) { return ''; } +function isset_request_var(...$args) { return false; } +function get_request_var(...$args) { return null; } +function get_nfilter_request_var(...$args) { return null; } +function get_filter_request_var(...$args) { return null; } +function cacti_session_close() {} +function raise_message_javascript(...$args) {} +function array_rekey($rows, $key, $value) { + return array_column($rows, $value, $key); +} + +function __esc($text, ...$args) { return html_escape($text); } From 89f4c5ac2b077a5aa431d2c1131a8e14f4cfcc6b Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 19 Oct 2026 23:34:11 +1030 Subject: [PATCH 20/29] Recreate failed parallel requests and reject partial results --- includes/functions.php | 22 +++-- tests/Unit/FilterLifecycleRegressionTest.php | 13 +++ tests/Unit/FilterStatusTest.php | 25 ++++++ tests/bootstrap-unit.php | 6 ++ tests/fixtures/parallel-retry.php | 85 ++++++++++++++++++++ 5 files changed, 146 insertions(+), 5 deletions(-) create mode 100644 tests/fixtures/parallel-retry.php diff --git a/includes/functions.php b/includes/functions.php index ed0fcce..3ce56b9 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -4266,7 +4266,7 @@ function parallel_database_query_run($requests) { raise_message_javascript('parallel_query_timeout', __('The Parallel Query Timed Out. Please contact your administrator', 'flowview'), MESSAGE_LEVEL_ERROR); foreach($requests as $request_id) { - parallel_database_query_cleanup($request_id); + parallel_database_query_cleanup($request_id, true); } return false; @@ -4288,9 +4288,20 @@ function parallel_database_query_run($requests) { FROM parallel_database_query WHERE id IN(' . implode(', ', $requests) . ')'); - // Missing reduction metadata is a failed report, not a successful empty result. + $valid_results = is_array($tables) && count($tables) === count($requests); + if ($valid_results) { + foreach ($tables as $index => $data) { + $tables[$index]['results'] = json_decode($data['results'] ?: '', true); + if (!is_array($tables[$index]['results'])) { + $valid_results = false; + break; + } + } + } + + // Every component must succeed before any results are materialized or reduced. if (is_array($query) && isset($query['sql_query'], $query['sql_where'], $query['sql_groupby'], $query['sql_having'], $query['sql_order'], $query['sql_limit'], $query['sql_params']) - && is_array($query['sql_params']) && is_array($tables) && count($tables) === count($requests)) { + && is_array($query['sql_params']) && $valid_results) { $tables = array_rekey($tables, 'map_table', ['map_create', 'results']); $i = 0; $temp_table = ''; @@ -4305,7 +4316,7 @@ function parallel_database_query_run($requests) { flowview_db_execute($create); } - $results = json_decode($data['results'], true); + $results = $data['results']; if (cacti_sizeof($results)) { $sql = []; @@ -4362,7 +4373,8 @@ function parallel_database_query_run($requests) { } foreach($requests as $request_id) { - parallel_database_query_cleanup($request_id); + // Failed cached requests must be recreated, with new shards, on retry. + parallel_database_query_cleanup($request_id, !is_array($results)); } return $results; diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index f3aa7c6..d2aebfd 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -157,3 +157,16 @@ $config['base_path'] = $oldBase; } })->with([false, true]); + +it('recreates shards and launches the same query after a parallel timeout', function () { + if (!extension_loaded('pdo_sqlite')) { + $this->markTestSkipped('PDO SQLite is needed for the persisted parallel retry regression'); + } + [$exit, $output] = ($this->runFixture)([PHP_BINARY, dirname(__DIR__) . '/fixtures/parallel-retry.php', dirname(__DIR__, 2), $this->fixtureRoot]); + $result = json_decode($output, true); + expect($exit)->toBe(0)->and($result['timeout'])->toBeFalse() + ->and($result['remaining'])->toBe(0)->and($result['remainingShards'])->toBe(0) + ->and($result['retry'])->not->toBe($result['first'])->and($result['retryShards'])->toBe(1) + ->and($result['result'])->toBe([])->and($result['launches'])->toBe([$result['first'], $result['retry']]) + ->and($result['cached'])->toBe($result['retry'])->and($result['cachedResult'])->toBe([]); +}); diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index 24e4767..d25a64d 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -197,6 +197,8 @@ expect(parallel_database_query_run([71, 72]))->toBe($success ? [] : false); $cleanup = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM parallel_database_query_shard'))); expect(array_column($cleanup, 'params'))->toBe([[71], [72]]); + $removed = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => preg_match('/^DELETE FROM parallel_database_query\s+WHERE/', $c['sql']))); + expect(array_column($removed, 'params'))->toBe($success ? [] : [[71], [72]]); })->with([[false, false], ['', false], ['null', false], ['bad-json', false], ['valid', true], ['valid', false, false], ['valid', false, []], ['{}', false]]); it('returns a failed result after cleaning up timed out parallel requests', function () { @@ -209,3 +211,26 @@ $cleanup = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM parallel_database_query_shard'))); expect(array_column($cleanup, 'params'))->toBe([[71], [72]]); }); + +it('rejects a failed component before reducing any two-request results', function ($failed, $position) { + $GLOBALS['debug'] = false; + $GLOBALS['__test_config']['flowview_parallel_runlimit'] = 5; + $query = ['sql_query' => 'SELECT SUM(bytes) bytes', 'sql_where' => '', 'sql_groupby' => '', 'sql_having' => '', 'sql_order' => '', 'sql_limit' => '', 'sql_params' => []]; + foreach ([0, 0, json_encode($query), '', ''] as $value) { + plugin_test_queue_db_result('db_fetch_cell_prepared', $value); + } + plugin_test_queue_db_result('db_fetch_cell', 0); + $tables = [ + ['map_table' => 'fixture_map_71', 'map_create' => 'CREATE TABLE fixture_map_71 (bytes bigint)', 'results' => '[{"bytes":123}]'], + ['map_table' => 'fixture_map_72', 'map_create' => 'CREATE TABLE fixture_map_72 (bytes bigint)', 'results' => '[{"bytes":456}]'], + ]; + $tables[$position]['results'] = $failed; + plugin_test_queue_db_result('db_fetch_assoc', $tables); + expect(parallel_database_query_run([71, 72]))->toBeFalse(); + $cleanup = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM parallel_database_query_shard'))); + expect(array_column($cleanup, 'params'))->toBe([[71], [72]]); + $removed = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => preg_match('/^DELETE FROM parallel_database_query\s+WHERE/', $c['sql']))); + expect(array_column($removed, 'params'))->toBe([[71], [72]]); + $reductions = array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_execute' || ($c['fn'] === 'db_fetch_assoc_prepared' && str_contains($c['sql'], 'SUM(bytes)'))); + expect($reductions)->toBe([]); +})->with(['false', 'null', 'invalid-json'])->with([0, 1]); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 7af683e..1164452 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -122,6 +122,12 @@ function plugin_test_db_result($function, $default) { } } +if (!function_exists('db_qstr')) { + function db_qstr($value) { + return "'" . str_replace("'", "''", (string) $value) . "'"; + } +} + if (!function_exists('db_execute')) { function db_execute($sql, $log = true, $conn = false) { $GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute', 'sql' => $sql, 'params' => array(), 'conn' => $conn); diff --git a/tests/fixtures/parallel-retry.php b/tests/fixtures/parallel-retry.php new file mode 100644 index 0000000..3930d06 --- /dev/null +++ b/tests/fixtures/parallel-retry.php @@ -0,0 +1,85 @@ + $argv[2]]; +$debug = false; +$db = new PDO('sqlite::memory:'); +$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); +$db->exec("CREATE TABLE parallel_database_query (id INTEGER PRIMARY KEY AUTOINCREMENT, md5sum TEXT, md5sum_tables TEXT, user_id INTEGER, total_shards INTEGER, map_query TEXT, map_range TEXT, map_range_params TEXT, reduce_query TEXT, created TEXT, time_to_live INTEGER, map_table TEXT, map_create TEXT, status TEXT DEFAULT 'pending', results TEXT)"); +$db->exec('CREATE TABLE parallel_database_query_shard (query_id INTEGER, shard_id INTEGER, full_scan INTEGER, map_table TEXT, map_partition TEXT, map_query TEXT, map_params TEXT)'); +$phase = 0; +$launches = []; +const MESSAGE_LEVEL_ERROR = 1; +const POLLER_VERBOSITY_MEDIUM = 2; +function __($text, ...$args) { return $text; } +function cacti_sizeof($value) { return is_array($value) ? count($value) : 0; } +function cacti_log(...$args) {} +function raise_message_javascript(...$args) {} +function read_config_option($name) { + return ['path_php_binary' => PHP_BINARY, 'flowview_parallel_runlimit' => $GLOBALS['phase'] ? 5 : 0, 'flowview_parallel_time_to_live' => 21600, 'flowview_engine' => 'Aria'][$name] ?? ''; +} +function array_rekey($rows, $key, $value) { + $result = []; + foreach ($rows as $row) { + $result[$row[$key]] = is_array($value) ? array_intersect_key($row, array_flip($value)) : $row[$value]; + } + return $result; +} +function db_fetch_assoc_prepared($sql, $params = [], ...$args) { + if (str_contains($sql, 'FROM processes')) { + return []; + } + if (str_contains($sql, 'INFORMATION_SCHEMA.COLUMNS')) { + return [['COLUMN_NAME' => 'bytes', 'DATA_TYPE' => 'bigint', 'COLUMN_TYPE' => 'bigint', 'IS_NULLABLE' => 'YES', 'COLUMN_DEFAULT' => '']]; + } + $stmt = $GLOBALS['db']->prepare($sql); + $stmt->execute($params); + return $stmt->fetchAll(PDO::FETCH_ASSOC); +} +function db_fetch_assoc($sql, ...$args) { + return str_contains($sql, 'information_schema.ENGINES') ? [['ENGINE' => 'Aria']] : db_fetch_assoc_prepared($sql); +} +function db_fetch_cell_prepared($sql, $params = [], ...$args) { + $rows = db_fetch_assoc_prepared($sql, $params); + return $rows ? reset($rows[0]) : false; +} +function db_fetch_cell($sql, ...$args) { return db_fetch_cell_prepared($sql); } +function db_execute_prepared($sql, $params = [], ...$args) { + $stmt = $GLOBALS['db']->prepare($sql); + return $stmt->execute($params); +} +function db_execute($sql, ...$args) { + $sql = preg_replace('/\) ENGINE=.*$/s', ')', $sql); + return db_execute_prepared($sql); +} +function sql_save($row, $table, ...$args) { + unset($row['id']); + $sql = 'INSERT INTO ' . $table . ' (' . implode(',', array_keys($row)) . ') VALUES (' . implode(',', array_fill(0, count($row), '?')) . ')'; + db_execute_prepared($sql, array_values($row)); + return (int) $GLOBALS['db']->lastInsertId(); +} +function exec_background($php, $command, ...$args) { + preg_match('/--query-id=(\d+)/', $command, $match); + $id = (int) $match[1]; + $GLOBALS['launches'][] = $id; + if ($GLOBALS['phase']) { + db_execute_prepared('UPDATE parallel_database_query SET status=?,results=? WHERE id=?', ['complete', '[]', $id]); + } +} +require $argv[1] . '/includes/database.php'; +require $argv[1] . '/includes/functions.php'; +$tables = ['fixture_raw' => ['min_date' => '2024-01-01 00:00:00', 'max_date' => '2024-01-01 01:00:00', 'table_name' => 'fixture_raw', 'table_partition' => '']]; +$inner = ['sql_query' => 'SELECT SUM(bytes) bytes', 'sql_where' => '', 'sql_range' => 'start_time >= ? AND start_time < ?', 'sql_range_params' => [1704067200, 1704070800], 'sql_params' => [], 'sql_start_time' => 1704067200, 'sql_end_time' => 1704070800]; +$outer = ['sql_query' => 'SELECT SUM(bytes) bytes', 'sql_where' => '', 'sql_groupby' => '', 'sql_having' => '', 'sql_order' => '', 'sql_limit' => '', 'sql_params' => []]; +$first = parallel_database_query_request($tables, $inner, $outer); +$timeout = parallel_database_query_run([$first]); +$remaining = db_fetch_cell('SELECT COUNT(*) FROM parallel_database_query'); +$remainingShards = db_fetch_cell('SELECT COUNT(*) FROM parallel_database_query_shard'); +$phase = 1; +$retry = parallel_database_query_request($tables, $inner, $outer); +$retryShards = db_fetch_cell_prepared('SELECT COUNT(*) FROM parallel_database_query_shard WHERE query_id=?', [$retry]); +$result = parallel_database_query_run([$retry]); +$cached = parallel_database_query_request($tables, $inner, $outer); +$cachedResult = parallel_database_query_run([$cached]); +echo json_encode(compact('first', 'timeout', 'remaining', 'remainingShards', 'retry', 'retryShards', 'result', 'cached', 'cachedResult', 'launches')); From bd9702a77d10b27cc941cb1fcd283ebb887c642e Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:03:06 +1030 Subject: [PATCH 21/29] Retry completed report cleanup without resending delivery --- includes/functions.php | 9 ++++++++- tests/Unit/FilterQueueBatchTest.php | 25 +++++++++++++++++++++++++ tests/Unit/FilterScheduleRetryTest.php | 2 +- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/includes/functions.php b/includes/functions.php index 3ce56b9..29ab740 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -7461,7 +7461,14 @@ function flowview_reports_run(int $id): bool { } return false; } - return db_execute_prepared('DELETE FROM reports_queued WHERE id=? AND source=?', [$id, 'flowview']) !== false; + // Delivery succeeded: retry only cleanup, never preparation or notification. + for ($attempt = 0; $attempt < 2; $attempt++) { + if (db_execute_prepared('DELETE FROM reports_queued WHERE id=? AND source=?', [$id, 'flowview']) !== false) { + return true; + } + } + cacti_log('Unable to remove completed FlowView occurrence ' . $id . ' for schedule ' . (int) $report['source_id'] . '; queue repair is required. The report already completed; remove this reports_queued row without rerunning it.', false, 'FLOWVIEW'); + return false; } /** Attempt a bounded, fair queue snapshot without retrying failures in the same pass. */ diff --git a/tests/Unit/FilterQueueBatchTest.php b/tests/Unit/FilterQueueBatchTest.php index 17af5a3..551bd25 100644 --- a/tests/Unit/FilterQueueBatchTest.php +++ b/tests/Unit/FilterQueueBatchTest.php @@ -136,3 +136,28 @@ expect($writes)->toHaveCount(1)->and($writes[0]['sql'])->toContain('AND status=?') ->and($writes[0]['params'][4])->toBe('pending'); }); + +it('retries successful completion cleanup without rerunning or requeuing the delivered report', function ($firstDelete, $retryDelete) { + $GLOBALS['__test_cacti_log'] = []; + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); + plugin_test_queue_db_result('exec_with_timeout', 0); + foreach ([true, $firstDelete, $retryDelete] as $result) { + plugin_test_queue_db_result('db_execute_prepared', $result); + } + expect(flowview_reports_run(51))->toBe($firstDelete || $retryDelete); + $calls = $GLOBALS['__test_db_calls']; + $exec = array_filter($calls, fn ($c) => $c['fn'] === 'exec_with_timeout'); + $writes = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'db_execute_prepared')); + expect($exec)->toHaveCount(1)->and($writes)->toHaveCount($firstDelete ? 2 : 3); + foreach (array_slice($writes, 1) as $write) { + expect($write['sql'])->toBe('DELETE FROM reports_queued WHERE id=? AND source=?') + ->and($write['params'])->toBe([51, 'flowview']); + } + $log = implode(' ', $GLOBALS['__test_cacti_log']); + if (!$firstDelete && !$retryDelete) { + expect($log)->toContain('queue repair is required', '51', '7', 'already completed'); + } else { + expect($log)->not->toContain('queue repair is required'); + } +})->with([[true, true], [false, true], [false, false]]); diff --git a/tests/Unit/FilterScheduleRetryTest.php b/tests/Unit/FilterScheduleRetryTest.php index 98a5198..267ba0a 100644 --- a/tests/Unit/FilterScheduleRetryTest.php +++ b/tests/Unit/FilterScheduleRetryTest.php @@ -57,7 +57,7 @@ rmdir($root . '/lib'); rmdir($root); } -})->with([[75, true, false, false], [75, false, false, false], [1, true, false, false], [0, true, false, true], [0, true, false, false], [0, false, false, false], [75, true, true, false], [75, false, true, false], [0, true, true, false]]); +})->with([[75, true, false, false], [75, false, false, false], [1, true, false, false], [0, true, false, true], [0, true, false, false], [75, true, true, false], [75, false, true, false], [0, true, true, false]]); it('rejects missing queue rows or failed claims without running a child', function ($missing) { global $config; plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); From ceff2b45d93a93c4a374ee98436113502971f363 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:19:58 +1030 Subject: [PATCH 22/29] Reject partial parallel reports when materialization fails --- includes/functions.php | 13 ++++++++++--- tests/Unit/FilterStatusTest.php | 26 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/includes/functions.php b/includes/functions.php index 29ab740..bb6db8a 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -4305,6 +4305,7 @@ function parallel_database_query_run($requests) { $tables = array_rekey($tables, 'map_table', ['map_create', 'results']); $i = 0; $temp_table = ''; + $materialized = true; foreach($tables as $table => $data) { if ($i == 0) { @@ -4313,7 +4314,10 @@ function parallel_database_query_run($requests) { $create = str_replace($table, $temp_table, $data['map_create']); $create = str_replace('CREATE TABLE', 'CREATE TEMPORARY TABLE', $create); - flowview_db_execute($create); + if (flowview_db_execute($create) === false) { + $materialized = false; + break; + } } $results = $data['results']; @@ -4346,7 +4350,10 @@ function parallel_database_query_run($requests) { //cacti_log($sql_prefix . implode(', ', $sql)); /* insert entries into the intermediary table */ - flowview_db_execute($sql_prefix . implode(', ', $sql)); + if (flowview_db_execute($sql_prefix . implode(', ', $sql)) === false) { + $materialized = false; + break; + } } $i++; @@ -4362,7 +4369,7 @@ function parallel_database_query_run($requests) { //cacti_log($final_query); - $results = flowview_db_fetch_assoc_prepared($final_query, $query['sql_params']); + $results = $materialized ? flowview_db_fetch_assoc_prepared($final_query, $query['sql_params']) : false; //cacti_log(json_encode($results)); diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index d25a64d..d5a575b 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -234,3 +234,29 @@ $reductions = array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_execute' || ($c['fn'] === 'db_fetch_assoc_prepared' && str_contains($c['sql'], 'SUM(bytes)'))); expect($reductions)->toBe([]); })->with(['false', 'null', 'invalid-json'])->with([0, 1]); + +it('fails two-request materialization rather than reducing partial data', function ($writes, $success, $insertCount) { + $GLOBALS['debug'] = false; + $GLOBALS['__test_config']['flowview_parallel_runlimit'] = 5; + $query = ['sql_query' => 'SELECT SUM(bytes) bytes', 'sql_where' => '', 'sql_groupby' => '', 'sql_having' => '', 'sql_order' => '', 'sql_limit' => '', 'sql_params' => []]; + foreach ([0, 0, json_encode($query), '', ''] as $value) { + plugin_test_queue_db_result('db_fetch_cell_prepared', $value); + } + plugin_test_queue_db_result('db_fetch_cell', 0); + plugin_test_queue_db_result('db_fetch_assoc', [ + ['map_table' => 'fixture_map_71', 'map_create' => 'CREATE TABLE fixture_map_71 (bytes bigint)', 'results' => '[{"bytes":123}]'], + ['map_table' => 'fixture_map_72', 'map_create' => 'CREATE TABLE fixture_map_72 (bytes bigint)', 'results' => '[{"bytes":456}]'], + ]); + foreach ($writes as $result) { + plugin_test_queue_db_result('db_execute', $result); + } + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['bytes' => 579]]); + expect(parallel_database_query_run([71, 72]))->toBe($success ? [['bytes' => 579]] : false); + $calls = $GLOBALS['__test_db_calls']; + $inserts = array_filter($calls, fn ($c) => $c['fn'] === 'db_execute' && str_starts_with($c['sql'], 'INSERT INTO')); + $reductions = array_filter($calls, fn ($c) => $c['fn'] === 'db_fetch_assoc_prepared' && str_contains($c['sql'], 'SUM(bytes)')); + $drops = array_filter($calls, fn ($c) => $c['fn'] === 'db_execute' && str_starts_with($c['sql'], 'DROP TEMPORARY TABLE IF EXISTS')); + $removed = array_values(array_filter($calls, fn ($c) => preg_match('/^DELETE FROM parallel_database_query\s+WHERE/', $c['sql']))); + expect($inserts)->toHaveCount($insertCount)->and($reductions)->toHaveCount($success ? 1 : 0) + ->and($drops)->toHaveCount(1)->and(array_column($removed, 'params'))->toBe($success ? [] : [[71], [72]]); +})->with([[[false], false, 0], [[true, false], false, 1], [[true, true, false], false, 2], [[true, true, true], true, 2]]); From 716acbed55db1a11ae0d2bdadf73605d2411035a Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:31:52 +1030 Subject: [PATCH 23/29] Use child exit status to avoid resending reports with warnings --- includes/functions-pre13.php | 6 +++--- includes/functions.php | 4 ++-- tests/Unit/FilterLifecycleRegressionTest.php | 10 ++++++++++ tests/Unit/FilterScheduleRetryTest.php | 10 ++++++---- tests/bootstrap-unit.php | 5 ++++- tests/fixtures/report-stderr.php | 14 ++++++++++++++ 6 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 tests/fixtures/report-stderr.php diff --git a/includes/functions-pre13.php b/includes/functions-pre13.php index 1355c69..4b581db 100644 --- a/includes/functions-pre13.php +++ b/includes/functions-pre13.php @@ -282,14 +282,14 @@ function reports_run($id): bool { $start = microtime(true); - $return_code = 0; + $return_code = 1; $output = []; $command = $report['run_command'] . " --report-id=$id"; $timeout = $report['run_timeout']; cacti_log("The report:$id has command was:$command"); - $last_line = exec_with_timeout($command, $output, $return_code, $timeout); + exec_with_timeout($command, $output, $return_code, $timeout); $end = microtime(true); @@ -297,7 +297,7 @@ function reports_run($id): bool { cacti_log($stats, false, 'SYSTEM'); - if ($report['source'] === 'flowview' && ($last_line === false || $return_code !== 0)) { + if ($report['source'] === 'flowview' && $return_code !== 0) { if (!db_execute_prepared('UPDATE reports_queued SET status = ? WHERE id = ?', ['pending', $id])) { cacti_log('Unable to reset the retained FlowView occurrence; queue repair is required.', false, 'FLOWVIEW'); } diff --git a/includes/functions.php b/includes/functions.php index bb6db8a..027ff8b 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -7460,8 +7460,8 @@ function flowview_reports_run(int $id): bool { if ($timeout <= 0) { $timeout = max(1, (int) read_config_option('scheduler_timeout') ?: 300); } - $result = exec_with_timeout($command, $output, $status, $timeout); - if ($result === false || $status !== 0) { + exec_with_timeout($command, $output, $status, $timeout); + if ($status !== 0) { // Keep the original even if resetting its state fails; never delete it. if (!db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=? AND source=?', ['pending', $id, 'flowview'])) { cacti_log('Unable to reset the retained FlowView occurrence; queue repair is required.', false, 'FLOWVIEW'); diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index d2aebfd..d94893a 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -170,3 +170,13 @@ ->and($result['result'])->toBe([])->and($result['launches'])->toBe([$result['first'], $result['retry']]) ->and($result['cached'])->toBe($result['retry'])->and($result['cachedResult'])->toBe([]); }); + +it('completes an exit-zero report with stderr using Cactis actual process helper', function ($runner) { + global $config; + file_put_contents($this->fixtureRoot . '/warning.php', 'runFixture)([PHP_BINARY, dirname(__DIR__) . '/fixtures/report-stderr.php', $config['base_path'], dirname(__DIR__, 2), $this->fixtureRoot, $runner]); + $result = json_decode($output, true); + expect($exit)->toBe(0)->and($result['helperResult'])->toBeFalse()->and($result['status'])->toBe(0) + ->and($result['success'])->toBeTrue()->and($result['writes'])->toHaveCount(2) + ->and($result['writes'][1]['sql'])->toStartWith('DELETE FROM reports_queued'); +})->with(['native', 'legacy']); diff --git a/tests/Unit/FilterScheduleRetryTest.php b/tests/Unit/FilterScheduleRetryTest.php index 267ba0a..3421b5b 100644 --- a/tests/Unit/FilterScheduleRetryTest.php +++ b/tests/Unit/FilterScheduleRetryTest.php @@ -18,7 +18,7 @@ expect($legacy ? reports_run(51) : flowview_reports_run(51))->toBeFalse(); expect(array_column($GLOBALS['__test_db_calls'], 'fn'))->not->toContain('db_execute_prepared', 'exec_with_timeout'); })->with([false, true]); -it('retains the original on deferred or failed children and only deletes successful occurrences', function ($status, $reset, $legacy, $startup) { +it('retains the original on deferred or failed children and only deletes successful occurrences', function ($status, $reset, $legacy, $startup, $warning = false) { global $config; $old = $config['base_path']; $root = sys_get_temp_dir() . '/flowview-shim-' . bin2hex(random_bytes(8)); @@ -28,8 +28,8 @@ $config['base_path'] = $root; plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'name' => 'Fixture', 'source_id' => 7, 'run_command' => 'fixture-command', 'run_timeout' => $status === 1 ? 0 : 60]); - plugin_test_queue_db_result('exec_with_timeout', $status); - plugin_test_queue_db_result('exec_with_timeout_result', $startup ? false : null); + plugin_test_queue_db_result('exec_with_timeout', $startup ? null : $status); + plugin_test_queue_db_result('exec_with_timeout_result', ($startup || $warning) ? false : null); plugin_test_queue_db_result('db_execute_prepared', true); plugin_test_queue_db_result('db_execute_prepared', $reset); $success = $status === 0 && !$startup && $reset; @@ -44,6 +44,8 @@ expect($sql)->toContain('UPDATE reports_queued SET status'); } else { expect($sql)->toContain('DELETE FROM reports_queued'); + $writes = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'db_execute_prepared')); + expect($writes)->toHaveCount(2)->and($writes[1]['sql'])->toStartWith('DELETE FROM reports_queued'); } if (!$legacy) { $exec = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'exec_with_timeout'))[0]; @@ -57,7 +59,7 @@ rmdir($root . '/lib'); rmdir($root); } -})->with([[75, true, false, false], [75, false, false, false], [1, true, false, false], [0, true, false, true], [0, true, false, false], [75, true, true, false], [75, false, true, false], [0, true, true, false]]); +})->with([[75, true, false, false], [75, false, false, false], [1, true, false, false], [0, true, false, true], [0, true, false, false], [75, true, true, false], [75, false, true, false], [0, true, true, false], [0, true, true, true], [0, true, false, false, true], [0, true, true, false, true]]); it('rejects missing queue rows or failed claims without running a child', function ($missing) { global $config; plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 1164452..6f8960d 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -505,7 +505,10 @@ function get_timespan(&$span, $now, $timespan, $weekday) { if (!function_exists('exec_with_timeout')) { function exec_with_timeout($command, &$output, &$status, $timeout = 5) { $GLOBALS['__test_db_calls'][] = ['fn' => 'exec_with_timeout', 'sql' => '', 'params' => [$command, $timeout]]; - $status = plugin_test_db_result('exec_with_timeout', 0); + $exit = plugin_test_db_result('exec_with_timeout', 0); + if ($exit !== null) { + $status = $exit; + } return plugin_test_db_result('exec_with_timeout_result', null); } } diff --git a/tests/fixtures/report-stderr.php b/tests/fixtures/report-stderr.php new file mode 100644 index 0000000..3a70a05 --- /dev/null +++ b/tests/fixtures/report-stderr.php @@ -0,0 +1,14 @@ + 'enabled']); +plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'name' => 'Fixture', 'source_id' => 7, 'run_command' => $command, 'run_timeout' => 5]); +$success = $argv[4] === 'native' ? flowview_reports_run(51) : reports_run(51); +$writes = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_execute_prepared')); +echo json_encode(compact('helperResult', 'status', 'success', 'writes')); From 509c93b1bd3b4c2e724e5cd36caac61a10d6d2ad Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:42:52 +1030 Subject: [PATCH 24/29] Release owned upgrade registrations on every exit path --- flowview_upgrade.php | 11 +++++++++-- tests/Unit/FilterLifecycleRegressionTest.php | 12 +++++++++--- tests/fixtures/filter-lifecycle.php | 5 ++++- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/flowview_upgrade.php b/flowview_upgrade.php index 8246da5..f70b6fa 100644 --- a/flowview_upgrade.php +++ b/flowview_upgrade.php @@ -82,13 +82,21 @@ $info = plugin_flowview_version(); $current_version = $info['version']; -if (!register_process_start('flowview', 'upgrade', 1, 86400)) { +$registered = register_process_start('flowview', 'upgrade', 1, 86400); +if (!$registered) { if (empty($forcever)) { print 'WARNING: Running process detected. To override use --forcever' . PHP_EOL; exit(1); } } +if ($registered) { + $upgrade_pid = getmypid(); + register_shutdown_function(static function () use ($upgrade_pid) { + unregister_process('flowview', 'upgrade', 1, $upgrade_pid); + }); +} + if (!flowview_filter_status_upgrade()) { cacti_log('FlowView filter state upgrade failed; it will be retried.', true, 'FLOWVIEW'); exit(1); @@ -104,7 +112,6 @@ flowview_upgrade($current_version, $old_version); -unregister_process('flowview', 'upgrade', 1); exit(0); diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index d94893a..9c81f4b 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -60,15 +60,21 @@ expect($background)->toHaveCount(1)->and($background[0][1][1])->toContain('/plugins/flowview/flowview_upgrade.php'); } })->with(['flowview.php', 'flowview_filters.php', 'flowview_schedules.php'])->with([false, true]); -it('repairs same-version schemas only after background registration and reports repair failure', function ($mode, $code, $writes) { - [$exit, $output] = ($this->runFixture)([PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/flowview_upgrade.php'], $mode); +it('repairs same-version schemas only after background registration and reports repair failure', function ($mode, $code, $writes, $force = false) { + $command = [PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/flowview_upgrade.php']; + if ($force) { + $command[] = '--forcever=fixture-version'; + } + [$exit, $output] = ($this->runFixture)($command, $mode); $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); expect($result)->toBeArray($output); $actions = array_column($result['calls'], 0); expect($exit)->toBe($code)->and($result['repair_loaded'])->toBeTrue() ->and($actions[0])->toBe('register') ->and(array_filter($actions, fn ($action) => $action === 'ddl'))->toHaveCount($writes); -})->with([['pending', 0, 1], ['failed', 1, 1], ['ready', 0, 0], ['busy', 1, 0]]); + $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); + expect($released)->toBe($mode === 'busy' ? [] : [['unregister', ['flowview', 'upgrade', 1, $result['pid']]]]); +})->with([['pending', 0, 1], ['failed', 1, 1], ['ready', 0, 0], ['busy', 1, 0], ['pending', 0, 1, true], ['busy', 0, 1, true]]); it('retains a full failing snapshot while advancing to healthy rows on the next pass', function () { if (!extension_loaded('pdo_sqlite')) { $this->markTestSkipped('PDO SQLite is needed for the persisted queue regression'); diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php index 6d97505..3e0445b 100644 --- a/tests/fixtures/filter-lifecycle.php +++ b/tests/fixtures/filter-lifecycle.php @@ -64,8 +64,11 @@ function set_default_action() { $GLOBALS['calls'][] = ['page-ready']; exit; } +// Capture state after shutdown callbacks registered by the actual entrypoint. register_shutdown_function(function () { - file_put_contents(getenv('FLOWVIEW_FIXTURE_RESULT'), json_encode(['calls' => $GLOBALS['calls'], 'status' => http_response_code(), 'repair_loaded' => function_exists('flowview_filter_status_upgrade')])); + register_shutdown_function(function () { + file_put_contents(getenv('FLOWVIEW_FIXTURE_RESULT'), json_encode(['calls' => $GLOBALS['calls'], 'status' => http_response_code(), 'repair_loaded' => function_exists('flowview_filter_status_upgrade'), 'pid' => getmypid()])); + }); }); function db_fetch_row_prepared($sql, $params = [], ...$args) { $GLOBALS['calls'][] = ['row', $sql, $params]; From 72c610433aea3e62377003d342f63d042ac45ab8 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 20 Oct 2026 01:17:02 +1030 Subject: [PATCH 25/29] Reconcile report child outcomes before retrying timed out work --- README.md | 15 +++++ flowview_upgrade.php | 1 - includes/functions-pre13.php | 10 +--- includes/functions.php | 49 +++++++++++++--- run_schedule.php | 43 +++++++++----- tests/README.md | 9 +++ tests/Unit/FilterLifecycleRegressionTest.php | 53 ++++++++++++++++-- tests/Unit/FilterQueueBatchTest.php | 59 +++++++++++++++++++- tests/Unit/FilterScheduleRetryTest.php | 9 +-- tests/fixtures/filter-lifecycle.php | 8 +-- tests/fixtures/filter-queue.php | 14 ++++- tests/fixtures/report-timeout.php | 47 ++++++++++++++++ 12 files changed, 269 insertions(+), 48 deletions(-) create mode 100644 tests/fixtures/report-timeout.php diff --git a/README.md b/README.md index 222af62..9009439 100644 --- a/README.md +++ b/README.md @@ -223,3 +223,18 @@ Pending filter-state migration leaves scheduled queue entries and lastsent/start Queue dispatch processes a bounded snapshot, attempting each occurrence once per pass. A failed occurrence stays retained while healthy entries continue; previously attempted rows sort behind new work so repeated failures cannot monopolize the next snapshot. Schema loss or exhausted concurrency stops the pass. Scheduler capacity checks remove exact child registrations only when the operating system confirms that their PID no longer exists. Live or uncertain processes (including permission errors and platforms without POSIX liveness probes) retain their capacity reservation. Failed legacy queue resets now log an explicit queue-repair error while retaining the original occurrence. + + +FlowView uses one dispatcher on both supported Cacti versions. A child records +`complete` after successful preparation/delivery and saved schedule timestamps, +or `retry` only for a known pre-delivery deferral. These markers let a later +poller reconcile a child that finishes after its parent's timeout. Completed +occurrences are deleted without executing them again. Only pending work or an +explicit retry marker can start a new child. + +A timeout or crash with no recorded outcome leaves the occurrence `running` and +logs its queue ID instead of automatically resending it. If it remains unresolved, +check the child process and report/notification logs, then reconcile the specific +`reports_queued` row: remove an already-delivered occurrence only after repairing +its schedule timestamps, or return a confirmed undelivered occurrence to pending. +Do not reset an unresolved row merely because its process has disappeared. diff --git a/flowview_upgrade.php b/flowview_upgrade.php index f70b6fa..f02bc91 100644 --- a/flowview_upgrade.php +++ b/flowview_upgrade.php @@ -112,7 +112,6 @@ flowview_upgrade($current_version, $old_version); - exit(0); /** diff --git a/includes/functions-pre13.php b/includes/functions-pre13.php index 4b581db..72ae7a8 100644 --- a/includes/functions-pre13.php +++ b/includes/functions-pre13.php @@ -268,8 +268,8 @@ function reports_run($id): bool { [$id]); if (cacti_sizeof($report)) { - if ($report['source'] === 'flowview' && !flowview_filter_status_ready()) { - return false; + if ($report['source'] === 'flowview') { + return flowview_reports_run((int) $id, $report); } require_once($config['base_path'] . '/lib/poller.php'); db_execute_prepared('UPDATE reports_queued @@ -297,12 +297,6 @@ function reports_run($id): bool { cacti_log($stats, false, 'SYSTEM'); - if ($report['source'] === 'flowview' && $return_code !== 0) { - if (!db_execute_prepared('UPDATE reports_queued SET status = ? WHERE id = ?', ['pending', $id])) { - cacti_log('Unable to reset the retained FlowView occurrence; queue repair is required.', false, 'FLOWVIEW'); - } - return false; - } db_execute_prepared('DELETE FROM reports_queued WHERE id = ?', [$id]); return true; } diff --git a/includes/functions.php b/includes/functions.php index 027ff8b..554775c 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -7440,41 +7440,68 @@ function flowview_schedule_queue_id(array $options) { } /** Own FlowView dispatch so failed/deferred children cannot consume queue rows. */ -function flowview_reports_run(int $id): bool { +function flowview_reports_run(int $id, ?array $report = null): bool { global $config; if (!flowview_filter_status_ready()) { return false; } - $report = db_fetch_row_prepared('SELECT * FROM reports_queued WHERE id=? AND source=?', [$id, 'flowview']); + $report ??= db_fetch_row_prepared('SELECT * FROM reports_queued WHERE id=? AND source=?', [$id, 'flowview']); if (!$report) { return false; } + if (($report['status'] ?? '') === 'complete') { + return flowview_reports_complete($id, (int) $report['source_id']); + } + if (($report['status'] ?? '') === 'retry') { + if (!db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=? AND source=? AND status=?', ['pending', $id, 'flowview', 'retry'])) { + return false; + } + } require_once $config['base_path'] . '/lib/poller.php'; if (!db_execute_prepared('UPDATE reports_queued SET status=?, start_time=? WHERE id=? AND source=? AND status=?', ['running', date('Y-m-d H:i:s'), $id, 'flowview', 'pending']) || db_affected_rows() !== 1) { return false; } $output = []; - $status = 1; + $status = -2; // Sentinel: unchanged only when no process could be started. $command = $report['run_command'] . ' --report-id=' . (int) $report['source_id'] . ' --queue-id=' . $id; $timeout = (int) $report['run_timeout']; if ($timeout <= 0) { $timeout = max(1, (int) read_config_option('scheduler_timeout') ?: 300); } exec_with_timeout($command, $output, $status, $timeout); - if ($status !== 0) { - // Keep the original even if resetting its state fails; never delete it. - if (!db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=? AND source=?', ['pending', $id, 'flowview'])) { + $startup_failed = $status === -2; + if (!$startup_failed && $status !== 0 && $status !== 75) { + // A timeout/crash is not proof of failed delivery. The child may still finish. + $outcome = db_fetch_row_prepared('SELECT status FROM reports_queued WHERE id=? AND source=?', [$id, 'flowview']); + if (($outcome['status'] ?? '') === 'complete') { + return flowview_reports_complete($id, (int) $report['source_id']); + } + if (($outcome['status'] ?? '') !== 'retry') { + cacti_log('FlowView occurrence ' . $id . ' has an unresolved child outcome; retained without retry to prevent duplicate delivery. Check the child and reconcile reports_queued before retrying.', false, 'FLOWVIEW'); + return false; + } + $status = 75; + } + if ($startup_failed || $status === 75) { + // Never overwrite a retry another dispatcher has already claimed. + $expected = $startup_failed ? 'running' : 'retry'; + if (!db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=? AND source=? AND status=?', ['pending', $id, 'flowview', $expected])) { cacti_log('Unable to reset the retained FlowView occurrence; queue repair is required.', false, 'FLOWVIEW'); } return false; } + return flowview_reports_complete($id, (int) $report['source_id']); +} + +/** Delete a delivered occurrence without executing its child again. */ +function flowview_reports_complete(int $id, int $schedule_id): bool { // Delivery succeeded: retry only cleanup, never preparation or notification. for ($attempt = 0; $attempt < 2; $attempt++) { if (db_execute_prepared('DELETE FROM reports_queued WHERE id=? AND source=?', [$id, 'flowview']) !== false) { return true; } } - cacti_log('Unable to remove completed FlowView occurrence ' . $id . ' for schedule ' . (int) $report['source_id'] . '; queue repair is required. The report already completed; remove this reports_queued row without rerunning it.', false, 'FLOWVIEW'); + cacti_log('Unable to remove completed FlowView occurrence ' . $id . ' for schedule ' . $schedule_id . '; queue repair is required. The report already completed; remove this reports_queued row without rerunning it.', false, 'FLOWVIEW'); return false; } @@ -7485,7 +7512,7 @@ function flowview_reports_run_pending(int $concurrency = 1, int $limit = 100): i } // Previously attempted rows sort behind new work, so a full failed batch // cannot indefinitely starve occurrences beyond this snapshot's limit. - $reports = db_fetch_assoc_prepared('SELECT id FROM reports_queued WHERE source=? AND status=? ORDER BY start_time,scheduled_time,id LIMIT ' . max(1, min(1000, $limit)), ['flowview', 'pending']); + $reports = db_fetch_assoc_prepared('SELECT id,status FROM reports_queued WHERE source=? AND status IN (?, ?, ?) ORDER BY (status=?) DESC,start_time,scheduled_time,id LIMIT ' . max(1, min(1000, $limit)), ['flowview', 'pending', 'retry', 'complete', 'complete']); if (!is_array($reports)) { cacti_log('Unable to read pending FlowView occurrences.', false, 'FLOWVIEW'); return 0; @@ -7495,6 +7522,12 @@ function flowview_reports_run_pending(int $concurrency = 1, int $limit = 100): i if (!flowview_filter_status_ready()) { break; } + if (($report['status'] ?? '') === 'complete') { + if (flowview_reports_run((int) $report['id'])) { + $done++; + } + continue; + } $running = flowview_running_schedule_children(); if ($running === null || (int) $running >= max(1, $concurrency)) { break; diff --git a/run_schedule.php b/run_schedule.php index 51b6cc1..6cf314d 100644 --- a/run_schedule.php +++ b/run_schedule.php @@ -103,6 +103,10 @@ if (!register_process_start('flowsched', 'master', 0, 1200)) { exit(0); } + $master_pid = getmypid(); + register_shutdown_function(static function () use ($master_pid) { + unregister_process('flowsched', 'master', 0, $master_pid); + }); } $start = microtime(true); @@ -120,9 +124,6 @@ cacti_log(sprintf('FLOWVIEW REPORT STATS: Time:%0.2f Reports:%s', $end - $start, $run), false, 'SYSTEM'); - if (!$force) { - unregister_process('flowsched', 'master', 0); - } } else { $report_id = flowview_schedule_queue_id(['report-id' => $report_id, 'queue-id' => $queue_id]); if ($report_id === false) { @@ -130,6 +131,19 @@ exit(1); } + $id = null; + $outcome = null; + $child_pid = getmypid(); + $owns_registration = false; + register_shutdown_function(static function () use (&$outcome, &$owns_registration, $child_pid, &$id, $report_id) { + if ($outcome !== null && !db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=? AND source=? AND status=?', [$outcome, $report_id, 'flowview', 'running'])) { + cacti_log('Unable to record FlowView child outcome for occurrence ' . $report_id . '; queue repair is required. Do not resend without verifying delivery.', false, 'FLOWVIEW'); + } + if ($owns_registration) { + unregister_process('flowsched', "child_$id", 0, $child_pid); + } + }); + $report = db_fetch_row_prepared('SELECT rq.* FROM reports_queued AS rq WHERE rq.source = ? @@ -137,6 +151,7 @@ ['flowview', $report_id]); if ($report === false) { + $outcome = 'retry'; exit(75); } @@ -145,14 +160,17 @@ if (!flowview_filter_status_ready()) { // The FlowView parent retains this original occurrence on a retry exit. + $outcome = 'retry'; exit(75); } /* silently end if the registered process is still running, or process table missing */ if (!$force) { if (!register_process_start('flowsched', "child_$id", 0, 1200)) { + $outcome = 'retry'; exit(75); } + $owns_registration = true; } $t = time(); @@ -165,9 +183,7 @@ [$id]); if ($schedule === false) { - if (!$force) { - unregister_process('flowsched', "child_$id", 0); - } + $outcome = 'retry'; exit(75); } @@ -175,16 +191,17 @@ if (!plugin_flowview_run_schedule($id, $report_id)) { // A deferred or failed preparation must preserve the occurrence // and schedule timestamps just like the initial readiness check. - if (!$force) { - unregister_process('flowsched', "child_$id", 0); - } + $outcome = 'retry'; exit(75); } - flowview_db_execute_prepared('UPDATE plugin_flowview_schedules + if (!flowview_db_execute_prepared('UPDATE plugin_flowview_schedules SET lastsent = ?, start = ? WHERE id = ?', - array($r, date('Y-m-d H:i:s'), $id)); + array($r, date('Y-m-d H:i:s'), $id))) { + cacti_log('FlowView occurrence ' . $report_id . ' finished delivery but schedule timestamps could not be saved; queue repair is required. Verify delivery and repair the schedule before clearing the occurrence.', false, 'FLOWVIEW'); + exit(76); + } $end = microtime(true); @@ -193,9 +210,7 @@ cacti_log('FLOWVIEW SCHEDULE STATS: ' . $cacti_stats , true, 'SYSTEM'); } - if (!$force) { - unregister_process('flowsched', "child_$id", 0); - } + $outcome = 'complete'; } else { cacti_log("WARNING: Unable to find Report ID $report_id", false, 'REPORTS'); } diff --git a/tests/README.md b/tests/README.md index a744649..a478b0a 100644 --- a/tests/README.md +++ b/tests/README.md @@ -47,3 +47,12 @@ an in-memory SQLite database to check persisted ordering across multiple passes: a full failed batch remains pending while healthy rows advance on the next pass. This queue test requires PDO SQLite and skips explicitly when it is unavailable; it does not validate MariaDB schema syntax or metadata locking. + + +Timeout regressions load Cacti's actual process helper and persist queue state in +SQLite while a worker writes stderr and completes after the parent timeout. Both +legacy and native entrypoints must avoid requeueing and reconcile late completion +without a second delivery. Separate actual-child tests cover durable outcome +markers, PID-scoped shutdown cleanup, deferred preparation and timestamp-write +failure after completion. Fixtures adapt database APIs and worker delivery; they +do not test SMTP or replace live MariaDB concurrency validation. diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index 9c81f4b..d47ab4c 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -82,7 +82,8 @@ [$exit, $output] = ($this->runFixture)([PHP_BINARY, dirname(__DIR__) . '/fixtures/filter-queue.php', dirname(__DIR__, 2), $this->fixtureRoot]); $result = json_decode($output, true); expect($exit)->toBe(0)->and($result['first'])->toBe(0)->and($result['second'])->toBe(2)->and($result['third'])->toBe(0) - ->and($result['attempts'])->toBe([1, 2, 3, 4, 1, 2]) + ->and($result['attempts'])->toBe([1, 2, 3, 4, 1, 2, 6]) + ->and($result['retryRace'])->toBeFalse()->and($result['raceStatus'])->toBe(['status' => 'running']) ->and($result['lostClaim'])->toBeFalse()->and($result['claimedStatus'])->toBe(['status' => 'running']) ->and($result['afterFirst'])->toHaveCount(4) ->and($result['remaining'])->toBe([['id' => 1, 'status' => 'pending'], ['id' => 2, 'status' => 'pending']]); @@ -98,8 +99,9 @@ $writes = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'write')); $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); expect($output)->toBe('')->and($exit)->toBe($retry ? 75 : 0) - ->and($released)->toBe([['unregister', ['flowsched', 'child_7', 0]]]) - ->and($writes)->toHaveCount($retry ? 0 : 1); + ->and($released)->toBe([['unregister', ['flowsched', 'child_7', 0, $result['pid']]]]) + ->and($writes)->toHaveCount($retry ? 1 : 2); + expect(end($writes)[2])->toBe([$retry ? 'retry' : 'complete', 51, 'flowview', 'running']); if (!$retry) { expect($writes[0][1])->toContain('UPDATE plugin_flowview_schedules'); } @@ -128,10 +130,14 @@ $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); expect($exit)->toBe(75); $actions = array_column($result['calls'], 0); - expect($actions)->not->toContain('write'); + $writes = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'write')); + expect($writes)->toHaveCount(1); + if ($writes) { + expect($writes[0][2])->toBe(['retry', 51, 'flowview', 'running']); + } if ($mode !== 'report-read-failure') { $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); - expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0]]]); + expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0, $result['pid']]]]); } })->with(['report-read-failure', 'schedule-read-failure', 'filter-read-failure'])->with([false, true]); @@ -148,7 +154,7 @@ $cleanup = array_values(array_filter($writes, fn ($c) => str_starts_with($c[1], 'DELETE FROM parallel_database_query_shard'))); expect(array_column($cleanup, 2))->toBe([[71]]); $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); - expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0]]]); + expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0, $result['pid']]]]); global $config; $oldBase = $config['base_path']; $config['base_path'] = $this->fixtureRoot; @@ -186,3 +192,38 @@ ->and($result['success'])->toBeTrue()->and($result['writes'])->toHaveCount(2) ->and($result['writes'][1]['sql'])->toStartWith('DELETE FROM reports_queued'); })->with(['native', 'legacy']); + +it('reconciles a timeout with stderr and late completion without resending on either dispatcher', function ($runner) { + if (!extension_loaded('pdo_sqlite')) { + $this->markTestSkipped('PDO SQLite is needed for the persisted timeout regression'); + } + global $config; + [$exit, $output] = ($this->runFixture)([PHP_BINARY, dirname(__DIR__) . '/fixtures/report-timeout.php', $config['base_path'], dirname(__DIR__, 2), $this->fixtureRoot, $runner]); + $result = json_decode($output, true); + expect($exit)->toBe(0)->and($result['remaining'])->toBe([])->and($result['deliveries'])->toHaveCount(1); + $resets = array_filter($result['writes'], fn ($c) => ($c[1][0] ?? '') === 'pending'); + expect($resets)->toBe([]); + $claims = array_filter($result['writes'], fn ($c) => str_contains($c[0], 'start_time=?')); + expect($claims)->toHaveCount(1); +})->with(['native', 'legacy']); + +it('holds a delivered occurrence when schedule timestamp persistence fails', function ($native) { + $command = [PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/run_schedule.php', '--report-id=' . ($native ? 7 : 51)]; + if ($native) { $command[] = '--queue-id=51'; } + [$exit] = ($this->runFixture)($command, 'timestamp-failure'); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + expect($exit)->toBe(76); + $outcomes = array_filter($result['calls'], fn ($c) => $c[0] === 'write' && str_contains($c[1], 'UPDATE reports_queued')); + expect($outcomes)->toBe([]); + $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); + expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0, $result['pid']]]]); +})->with([false, true]); + +it('releases only an acquired scheduler master registration at shutdown', function ($mode, $force) { + $command = [PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/run_schedule.php', '--scheduled']; + if ($force) { $command[] = '--force'; } + [$exit] = ($this->runFixture)($command, $mode); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); + expect($exit)->toBe(0)->and($released)->toBe($mode === 'master-busy' || $force ? [] : [['unregister', ['flowsched', 'master', 0, $result['pid']]]]); +})->with([['ready', false], ['master-busy', false], ['ready', true]]); diff --git a/tests/Unit/FilterQueueBatchTest.php b/tests/Unit/FilterQueueBatchTest.php index 551bd25..277c883 100644 --- a/tests/Unit/FilterQueueBatchTest.php +++ b/tests/Unit/FilterQueueBatchTest.php @@ -33,7 +33,7 @@ $deletes = array_values(array_filter($calls, fn ($c) => str_starts_with($c['sql'], 'DELETE FROM reports_queued'))); expect($deletes)->toHaveCount(1)->and($deletes[0]['params'])->toBe([52, 'flowview']); $query = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'db_fetch_assoc_prepared'))[0]; - expect($query['sql'])->toContain('ORDER BY start_time,scheduled_time,id LIMIT 100'); + expect($query['sql'])->toContain('ORDER BY (status=?) DESC,start_time,scheduled_time,id LIMIT 100'); }); it('stops the batch when schema readiness is lost instead of starting another child', function () { foreach ([true, true, true, false] as $ready) { @@ -161,3 +161,60 @@ expect($log)->not->toContain('queue repair is required'); } })->with([[true, true], [false, true], [false, false]]); + +it('does not requeue an unresolved timeout or crash and reconciles only explicit child outcomes', function ($status, $outcome, $success, $reset) { + $GLOBALS['__test_cacti_log'] = []; + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 1]); + plugin_test_queue_db_result('db_fetch_row_prepared', $outcome); + plugin_test_queue_db_result('exec_with_timeout', $status); + plugin_test_queue_db_result('exec_with_timeout_result', false); + expect(flowview_reports_run(51))->toBe($success); + $writes = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_execute_prepared')); + expect($writes)->toHaveCount(($success || $reset) ? 2 : 1); + if ($reset) { + expect($writes[1]['params'])->toBe(['pending', 51, 'flowview', 'retry']); + } elseif ($success) { + expect($writes[1]['sql'])->toStartWith('DELETE FROM reports_queued'); + } else { + expect(implode(' ', $GLOBALS['__test_cacti_log']))->toContain('unresolved child outcome', '51'); + } +})->with([[-1, ['status' => 'running'], false, false], [-1, false, false, false], [255, [], false, false], [-1, ['status' => 'complete'], true, false], [-1, ['status' => 'retry'], false, true]]); + +it('reconciles late complete and retry markers without losing queue ownership', function ($state, $reset, $claim, $success, $launches) { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'status' => $state, 'run_command' => 'fixture', 'run_timeout' => 1]); + foreach ([$reset, $claim] as $value) { + plugin_test_queue_db_result('db_execute_prepared', $value); + } + expect(flowview_reports_run(51))->toBe($success); + expect(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'exec_with_timeout'))->toHaveCount($launches); +})->with([['complete', true, true, true, 0], ['retry', false, true, false, 0], ['retry', true, false, false, 0], ['retry', true, true, true, 1]]); + +it('cleans late completed reports even when execution capacity is full', function () { + for ($i = 0; $i < 3; $i++) { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + } + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 51, 'status' => 'complete']]); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'status' => 'complete']); + expect(flowview_reports_run_pending())->toBe(1); + $exec = array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'exec_with_timeout'); + expect($exec)->toHaveCount(0); + $reads = array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_fetch_assoc_prepared' && str_contains($c['sql'], 'FROM processes')); + expect($reads)->toHaveCount(0); +}); + +it('preserves legacy dispatch for other report sources', function () { + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'other-plugin', 'source_id' => 7, 'name' => 'Other report', 'run_command' => 'fixture', 'run_timeout' => 60]); + expect(reports_run(51))->toBeTrue(); + $exec = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'exec_with_timeout')); + expect($exec)->toHaveCount(1)->and($exec[0]['params'])->toBe(['fixture --report-id=51', 60]); +}); + +it('uses the scheduler default when a report timeout is unset', function () { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 0]); + expect(flowview_reports_run(51))->toBeTrue(); + $exec = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'exec_with_timeout')); + expect($exec[0]['params'][1])->toBe(300); +}); diff --git a/tests/Unit/FilterScheduleRetryTest.php b/tests/Unit/FilterScheduleRetryTest.php index 3421b5b..8d23218 100644 --- a/tests/Unit/FilterScheduleRetryTest.php +++ b/tests/Unit/FilterScheduleRetryTest.php @@ -59,7 +59,7 @@ rmdir($root . '/lib'); rmdir($root); } -})->with([[75, true, false, false], [75, false, false, false], [1, true, false, false], [0, true, false, true], [0, true, false, false], [75, true, true, false], [75, false, true, false], [0, true, true, false], [0, true, true, true], [0, true, false, false, true], [0, true, true, false, true]]); +})->with([[75, true, false, false], [75, false, false, false], [0, true, false, true], [0, true, false, false], [75, true, true, false], [75, false, true, false], [0, true, true, false], [0, true, true, true], [0, true, false, false, true], [0, true, true, false, true]]); it('rejects missing queue rows or failed claims without running a child', function ($missing) { global $config; plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); @@ -104,7 +104,7 @@ $result = var_export($root . '/result.json', true); $core = var_export($root, true); $readyCode = $ready ? 'plugin_test_queue_db_result("db_fetch_row", ["Field"=>"enabled"]);plugin_test_queue_db_result("register_process_start", false);' : ''; - file_put_contents($root . '/include/cli_check.php', '51,"source"=>"flowview","source_id"=>7]); register_shutdown_function(function(){file_put_contents(' . $result . ',json_encode($GLOBALS["__test_db_calls"]));});'); + file_put_contents($root . '/include/cli_check.php', '51,"source"=>"flowview","source_id"=>7]); register_shutdown_function(function(){register_shutdown_function(function(){file_put_contents(' . $result . ',json_encode($GLOBALS["__test_db_calls"]));});});'); try { $command = [PHP_BINARY, $root . '/plugins/flowview/run_schedule.php', '--report-id=' . ($native ? 7 : 51)]; if ($native) { @@ -119,8 +119,9 @@ $exit = proc_close($process); expect($error)->toBe('')->and($output)->toBe('')->and($exit)->toBe(75); $calls = json_decode(file_get_contents($root . '/result.json'), true); - expect($calls)->toHaveCount($ready ? 3 : 2)->and($calls[0]['params'])->toBe(['flowview', 51]); - expect(array_column($calls, 'fn'))->not->toContain('db_execute_prepared'); + expect($calls)->toHaveCount($ready ? 4 : 3)->and($calls[0]['params'])->toBe(['flowview', 51]); + $markers = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'db_execute_prepared')); + expect($markers)->toHaveCount(1)->and($markers[0]['params'])->toBe(['retry', 51, 'flowview', 'running']); if ($ready) { global $config; $oldBase = $config['base_path']; diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php index 3e0445b..91ced19 100644 --- a/tests/fixtures/filter-lifecycle.php +++ b/tests/fixtures/filter-lifecycle.php @@ -4,7 +4,7 @@ $config = ['base_path' => dirname(__DIR__), 'poller_id' => 1]; $local_db_cnn_id = 'fixture'; $mode = getenv('FLOWVIEW_FIXTURE_MODE'); -$ready = in_array($mode, ['ready', 'late', 'child-disabled', 'report-read-failure', 'schedule-read-failure', 'filter-read-failure', 'parallel-timeout'], true); +$ready = in_array($mode, ['ready', 'late', 'child-disabled', 'report-read-failure', 'schedule-read-failure', 'filter-read-failure', 'parallel-timeout', 'timestamp-failure', 'master-busy'], true); $metadataReads = 0; $calls = []; const MESSAGE_LEVEL_INFO = 1; @@ -19,7 +19,7 @@ function cacti_sizeof($value) { return is_array($value) ? count($value) : 0; } function read_config_option($name) { - return ['path_php_binary' => PHP_BINARY, 'flowview_parallel_threads' => 2, 'flowview_partition' => 1, 'flowview_parallel_time_to_live' => 21600, 'flowview_parallel_runlimit' => 0][$name] ?? ''; + return ['path_php_binary' => PHP_BINARY, 'reports_concurrent' => 1, 'flowview_parallel_threads' => 2, 'flowview_partition' => 1, 'flowview_parallel_time_to_live' => 21600, 'flowview_parallel_runlimit' => 0][$name] ?? ''; } function cacti_log(...$args) { } @@ -55,7 +55,7 @@ function exec_background(...$args) { } function register_process_start(...$args) { $GLOBALS['calls'][] = ['register', $args]; - return $GLOBALS['mode'] !== 'busy'; + return !in_array($GLOBALS['mode'], ['busy', 'master-busy'], true); } function unregister_process(...$args) { $GLOBALS['calls'][] = ['unregister', $args]; @@ -89,7 +89,7 @@ function db_fetch_row_prepared($sql, $params = [], ...$args) { } function db_execute_prepared($sql, $params = [], ...$args) { $GLOBALS['calls'][] = ['write', $sql, $params]; - return true; + return !($GLOBALS['mode'] === 'timestamp-failure' && str_contains($sql, 'UPDATE plugin_flowview_schedules')); } function db_fetch_assoc($sql, ...$args) { diff --git a/tests/fixtures/filter-queue.php b/tests/fixtures/filter-queue.php index 69ff3cc..1db5042 100644 --- a/tests/fixtures/filter-queue.php +++ b/tests/fixtures/filter-queue.php @@ -36,7 +36,14 @@ function exec_with_timeout($command, &$output, &$status, $timeout) { preg_match('/--queue-id=(\d+)/', $command, $match); $id = (int) $match[1]; $GLOBALS['attempts'][] = $id; - $status = $id <= 2 ? 75 : 0; + $status = $id <= 2 || $id === 6 ? 75 : 0; + if ($status === 75) { + db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=?', ['retry', $id]); + if ($id === 6) { + // Another dispatcher consumed the child's retry marker before this parent returned. + db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=?', ['running', $id]); + } + } return ''; } function read_config_option(...$args) { @@ -54,4 +61,7 @@ function cacti_log(...$args) { $db->exec("INSERT INTO reports_queued VALUES (5, 'flowview', 5, 'running', '2026-01-01 00:00:00', '2026-01-01 00:00:00', 'fixture', 1)"); $lostClaim = flowview_reports_run(5); $claimedStatus = db_fetch_row_prepared('SELECT status FROM reports_queued WHERE id=5'); -echo json_encode(compact('first', 'second', 'third', 'attempts', 'afterFirst', 'remaining', 'lostClaim', 'claimedStatus')); +$db->exec("INSERT INTO reports_queued VALUES (6, 'flowview', 6, 'pending', '2026-01-01 00:00:00', '2026-01-01 00:00:00', 'fixture', 1)"); +$retryRace = flowview_reports_run(6); +$raceStatus = db_fetch_row_prepared('SELECT status FROM reports_queued WHERE id=6'); +echo json_encode(compact('retryRace', 'raceStatus', 'first', 'second', 'third', 'attempts', 'afterFirst', 'remaining', 'lostClaim', 'claimedStatus')); diff --git a/tests/fixtures/report-timeout.php b/tests/fixtures/report-timeout.php new file mode 100644 index 0000000..7243118 --- /dev/null +++ b/tests/fixtures/report-timeout.php @@ -0,0 +1,47 @@ + $argv[1]]; +$dbPath = $argv[3] . '/queue.sqlite'; +$db = new PDO('sqlite:' . $dbPath); +$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); +$db->exec('CREATE TABLE reports_queued (id INTEGER PRIMARY KEY, source TEXT, source_id INTEGER, status TEXT, start_time TEXT, scheduled_time TEXT, run_command TEXT, run_timeout INTEGER)'); +$worker = $argv[3] . '/late.php'; +$delivery = $argv[3] . '/deliveries.txt'; +file_put_contents($worker, 'exec("UPDATE reports_queued SET status=\'complete\' WHERE id=51 AND status=\'running\'");'); +$command = escapeshellarg(PHP_BINARY) . ' ' . escapeshellarg($worker); +$stmt = $db->prepare("INSERT INTO reports_queued VALUES (51,'flowview',7,'pending','','',?,1)"); +$stmt->execute([$command]); +$writes = []; +$logs = []; +function db_fetch_row($sql, ...$args) { return ['Field' => 'enabled']; } +function db_fetch_assoc_prepared($sql, $params = [], ...$args) { + if (str_contains($sql, 'FROM processes')) { return []; } + $stmt = $GLOBALS['db']->prepare($sql); + $stmt->execute($params); + return $stmt->fetchAll(PDO::FETCH_ASSOC); +} +function db_fetch_row_prepared($sql, $params = [], ...$args) { return db_fetch_assoc_prepared($sql, $params)[0] ?? []; } +function db_execute_prepared($sql, $params = [], ...$args) { + $GLOBALS['writes'][] = [$sql, $params]; + $stmt = $GLOBALS['db']->prepare($sql); + $result = $stmt->execute($params); + $GLOBALS['affected'] = $stmt->rowCount(); + return $result; +} +function db_affected_rows(...$args) { return $GLOBALS['affected'] ?? 0; } +function cacti_sizeof($rows) { return is_array($rows) ? count($rows) : 0; } +function read_config_option(...$args) { return ''; } +function cacti_log($text, ...$args) { $GLOBALS['logs'][] = $text; } +require $argv[1] . '/lib/poller.php'; +require $argv[2] . '/includes/database.php'; +require $argv[2] . '/includes/functions.php'; +$initial = $argv[4] === 'native' ? flowview_reports_run(51) : reports_run(51); +$deadline = microtime(true) + 5; +while (!is_file($delivery) && microtime(true) < $deadline) { usleep(50000); } +// The delivery marker precedes the durable outcome; wait for that write too. +while ((db_fetch_row_prepared('SELECT status FROM reports_queued WHERE id=51')['status'] ?? '') === 'running' && microtime(true) < $deadline) { usleep(50000); } +$late = flowview_reports_run_pending(); +$remaining = db_fetch_assoc_prepared('SELECT * FROM reports_queued'); +$deliveries = is_file($delivery) ? file($delivery) : []; +echo json_encode(compact('initial', 'late', 'remaining', 'deliveries', 'writes', 'logs')); From 537dca19ef9481afef33d156dca61712249e20be Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 20 Oct 2026 08:43:00 +1030 Subject: [PATCH 26/29] Require recorded child outcomes and isolate completion cleanup budget --- README.md | 7 ++- includes/functions.php | 49 ++++++++++---------- tests/README.md | 5 ++ tests/Unit/FilterLifecycleRegressionTest.php | 32 +++++++++++++ tests/Unit/FilterQueueBatchTest.php | 39 +++++++++++++--- tests/Unit/FilterScheduleRetryTest.php | 2 + tests/bootstrap-unit.php | 6 +++ tests/fixtures/completion-budget.php | 46 ++++++++++++++++++ tests/fixtures/filter-lifecycle.php | 8 ++++ tests/fixtures/filter-queue.php | 3 ++ tests/fixtures/report-stderr.php | 1 + 11 files changed, 167 insertions(+), 31 deletions(-) create mode 100644 tests/fixtures/completion-budget.php diff --git a/README.md b/README.md index 9009439..6432e86 100644 --- a/README.md +++ b/README.md @@ -232,9 +232,14 @@ poller reconcile a child that finishes after its parent's timeout. Completed occurrences are deleted without executing them again. Only pending work or an explicit retry marker can start a new child. -A timeout or crash with no recorded outcome leaves the occurrence `running` and +Every started child requires a recorded outcome, including an exit code of zero. +A timeout, crash or early exit with no recorded outcome leaves the occurrence `running` and logs its queue ID instead of automatically resending it. If it remains unresolved, check the child process and report/notification logs, then reconcile the specific `reports_queued` row: remove an already-delivered occurrence only after repairing its schedule timestamps, or return a confirmed undelivered occurrence to pending. Do not reset an unresolved row merely because its process has disappeared. + +Completion cleanup and executable reports use separate bounded snapshots, so +persistent cleanup failures cannot consume the pending-work budget. Cleanup +still runs first and does not require a free execution slot. diff --git a/includes/functions.php b/includes/functions.php index 554775c..c345d25 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -7469,28 +7469,24 @@ function flowview_reports_run(int $id, ?array $report = null): bool { $timeout = max(1, (int) read_config_option('scheduler_timeout') ?: 300); } exec_with_timeout($command, $output, $status, $timeout); - $startup_failed = $status === -2; - if (!$startup_failed && $status !== 0 && $status !== 75) { - // A timeout/crash is not proof of failed delivery. The child may still finish. + $expected = 'running'; + if ($status !== -2) { + // Exit status alone cannot prove completion, even when it is zero. $outcome = db_fetch_row_prepared('SELECT status FROM reports_queued WHERE id=? AND source=?', [$id, 'flowview']); if (($outcome['status'] ?? '') === 'complete') { return flowview_reports_complete($id, (int) $report['source_id']); } if (($outcome['status'] ?? '') !== 'retry') { - cacti_log('FlowView occurrence ' . $id . ' has an unresolved child outcome; retained without retry to prevent duplicate delivery. Check the child and reconcile reports_queued before retrying.', false, 'FLOWVIEW'); + cacti_log('FlowView occurrence ' . $id . ' has an unresolved child outcome (exit ' . $status . '); retained without retry to prevent duplicate delivery. Check the child and reconcile reports_queued before retrying.', false, 'FLOWVIEW'); return false; } - $status = 75; + $expected = 'retry'; } - if ($startup_failed || $status === 75) { - // Never overwrite a retry another dispatcher has already claimed. - $expected = $startup_failed ? 'running' : 'retry'; - if (!db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=? AND source=? AND status=?', ['pending', $id, 'flowview', $expected])) { - cacti_log('Unable to reset the retained FlowView occurrence; queue repair is required.', false, 'FLOWVIEW'); - } - return false; + // Never overwrite a retry another dispatcher has already claimed. + if (!db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=? AND source=? AND status=?', ['pending', $id, 'flowview', $expected])) { + cacti_log('Unable to reset the retained FlowView occurrence; queue repair is required.', false, 'FLOWVIEW'); } - return flowview_reports_complete($id, (int) $report['source_id']); + return false; } /** Delete a delivered occurrence without executing its child again. */ @@ -7510,24 +7506,29 @@ function flowview_reports_run_pending(int $concurrency = 1, int $limit = 100): i if (!flowview_filter_status_ready()) { return 0; } - // Previously attempted rows sort behind new work, so a full failed batch - // cannot indefinitely starve occurrences beyond this snapshot's limit. - $reports = db_fetch_assoc_prepared('SELECT id,status FROM reports_queued WHERE source=? AND status IN (?, ?, ?) ORDER BY (status=?) DESC,start_time,scheduled_time,id LIMIT ' . max(1, min(1000, $limit)), ['flowview', 'pending', 'retry', 'complete', 'complete']); + $limit = max(1, min(1000, $limit)); + $done = 0; + // Cleanup has its own budget: persistent deletion failures cannot starve execution. + $completed = db_fetch_assoc_prepared('SELECT id,source_id FROM reports_queued WHERE source=? AND status=? ORDER BY start_time,id LIMIT ' . $limit, ['flowview', 'complete']); + if (is_array($completed)) { + foreach ($completed as $report) { + if (flowview_reports_complete((int) $report['id'], (int) $report['source_id'])) { + $done++; + } + } + } else { + cacti_log('Unable to read completed FlowView occurrences for cleanup.', false, 'FLOWVIEW'); + } + // Executable work has an independent bounded, fair snapshot. + $reports = db_fetch_assoc_prepared('SELECT id FROM reports_queued WHERE source=? AND status IN (?, ?) ORDER BY start_time,scheduled_time,id LIMIT ' . $limit, ['flowview', 'pending', 'retry']); if (!is_array($reports)) { cacti_log('Unable to read pending FlowView occurrences.', false, 'FLOWVIEW'); - return 0; + return $done; } - $done = 0; foreach ($reports as $report) { if (!flowview_filter_status_ready()) { break; } - if (($report['status'] ?? '') === 'complete') { - if (flowview_reports_run((int) $report['id'])) { - $done++; - } - continue; - } $running = flowview_running_schedule_children(); if ($running === null || (int) $running >= max(1, $concurrency)) { break; diff --git a/tests/README.md b/tests/README.md index a478b0a..fa15d2c 100644 --- a/tests/README.md +++ b/tests/README.md @@ -56,3 +56,8 @@ without a second delivery. Separate actual-child tests cover durable outcome markers, PID-scoped shutdown cleanup, deferred preparation and timestamp-write failure after completion. Fixtures adapt database APIs and worker delivery; they do not test SMTP or replace live MariaDB concurrency validation. + +Successful-dispatch doubles explicitly supply recorded child completion; an exit +code alone never synthesizes that outcome. Regressions exercise an actual child +connection failure that exits zero before its outcome callback, and a persistent +queue with 100 undeletable completed rows followed by executable work. diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index d47ab4c..99fb737 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -113,6 +113,7 @@ plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); plugin_test_queue_db_result('exec_with_timeout', $exit); + plugin_test_queue_db_result('report_outcome', ['status' => $exit === 0 ? 'complete' : 'retry']); expect(flowview_reports_run(51))->toBe(!$retry); $deletes = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM reports_queued'))); expect($deletes)->toHaveCount($retry ? 0 : 1); @@ -162,6 +163,7 @@ plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); plugin_test_queue_db_result('exec_with_timeout', $exit); + plugin_test_queue_db_result('report_outcome', ['status' => $exit === 0 ? 'complete' : 'retry']); expect(flowview_reports_run(51))->toBeFalse(); $deletes = array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM reports_queued')); expect($deletes)->toBe([]); @@ -227,3 +229,33 @@ $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); expect($exit)->toBe(0)->and($released)->toBe($mode === 'master-busy' || $force ? [] : [['unregister', ['flowsched', 'master', 0, $result['pid']]]]); })->with([['ready', false], ['master-busy', false], ['ready', true]]); + +it('retains a child connection failure even when the actual entrypoint exits zero', function ($native) { + file_put_contents($this->fixtureRoot . '/connection.php', 'fixtureRoot . '/plugins/flowview/run_schedule.php', '--report-id=' . ($native ? 7 : 51)]; + if ($native) { $command[] = '--queue-id=51'; } + [$exit] = ($this->runFixture)($command, 'connection-failure'); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + expect($exit)->toBe(0)->and(array_column($result['calls'], 0))->toContain('connect')->not->toContain('write', 'register'); + global $config; + $oldBase = $config['base_path']; + $config['base_path'] = $this->fixtureRoot; + try { + plugin_test_reset(); + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 1]); + plugin_test_queue_db_result('exec_with_timeout', $exit); + expect($native ? flowview_reports_run(51) : reports_run(51))->toBeFalse(); + $writes = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_execute_prepared')); + expect($writes)->toHaveCount(1)->and($writes[0]['params'][0])->toBe('running'); + } finally { $config['base_path'] = $oldBase; } +})->with([false, true]); + +it('runs pending work beyond a full snapshot of undeletable completed occurrences', function () { + if (!extension_loaded('pdo_sqlite')) { $this->markTestSkipped('PDO SQLite is needed for the bounded cleanup regression'); } + [$exit, $output] = ($this->runFixture)([PHP_BINARY, dirname(__DIR__) . '/fixtures/completion-budget.php', dirname(__DIR__, 2), $this->fixtureRoot]); + $result = json_decode($output, true); + expect($exit)->toBe(0)->and($result['done'])->toBe(1)->and($result['attempts'])->toBe([101]) + ->and($result['deletes'])->toHaveCount(201)->and($result['remaining'])->toHaveCount(100) + ->and(array_unique(array_column($result['remaining'], 'status')))->toBe(['complete']); +}); diff --git a/tests/Unit/FilterQueueBatchTest.php b/tests/Unit/FilterQueueBatchTest.php index 277c883..080df3c 100644 --- a/tests/Unit/FilterQueueBatchTest.php +++ b/tests/Unit/FilterQueueBatchTest.php @@ -25,15 +25,17 @@ plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => $id, 'source' => 'flowview', 'source_id' => $id - 44, 'run_command' => 'fixture', 'run_timeout' => 60]); } plugin_test_queue_db_result('exec_with_timeout', 75); + plugin_test_queue_db_result('report_outcome', ['status' => 'retry']); plugin_test_queue_db_result('exec_with_timeout', 0); + plugin_test_queue_db_result('report_outcome', ['status' => 'complete']); expect(flowview_reports_run_pending())->toBe(1); $calls = $GLOBALS['__test_db_calls']; $exec = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'exec_with_timeout')); expect($exec)->toHaveCount(2)->and($exec[0]['params'][0])->toContain('--queue-id=51')->and($exec[1]['params'][0])->toContain('--queue-id=52'); $deletes = array_values(array_filter($calls, fn ($c) => str_starts_with($c['sql'], 'DELETE FROM reports_queued'))); expect($deletes)->toHaveCount(1)->and($deletes[0]['params'])->toBe([52, 'flowview']); - $query = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'db_fetch_assoc_prepared'))[0]; - expect($query['sql'])->toContain('ORDER BY (status=?) DESC,start_time,scheduled_time,id LIMIT 100'); + $query = array_values(array_filter($calls, fn ($c) => $c['fn'] === 'db_fetch_assoc_prepared' && !str_starts_with($c['sql'], 'SELECT id,source_id')))[0]; + expect($query['sql'])->toContain('ORDER BY start_time,scheduled_time,id LIMIT 100'); }); it('stops the batch when schema readiness is lost instead of starting another child', function () { foreach ([true, true, true, false] as $ready) { @@ -42,6 +44,7 @@ plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 51], ['id' => 52]]); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); plugin_test_queue_db_result('exec_with_timeout', 75); + plugin_test_queue_db_result('report_outcome', ['status' => 'retry']); expect(flowview_reports_run_pending())->toBe(0); expect(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'exec_with_timeout'))->toHaveCount(1); }); @@ -78,7 +81,9 @@ plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => $id, 'source' => 'flowview', 'source_id' => $id - 44, 'run_command' => 'fixture', 'run_timeout' => 60]); } plugin_test_queue_db_result('exec_with_timeout', 75); + plugin_test_queue_db_result('report_outcome', ['status' => 'retry']); plugin_test_queue_db_result('exec_with_timeout', 0); + plugin_test_queue_db_result('report_outcome', ['status' => 'complete']); expect(flowview_reports_run_pending())->toBe(1); $calls = $GLOBALS['__test_db_calls']; $deletes = array_values(array_filter($calls, fn ($c) => str_starts_with($c['sql'], 'DELETE FROM processes'))); @@ -142,6 +147,7 @@ plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); plugin_test_queue_db_result('exec_with_timeout', 0); + plugin_test_queue_db_result('report_outcome', ['status' => 'complete']); foreach ([true, $firstDelete, $retryDelete] as $result) { plugin_test_queue_db_result('db_execute_prepared', $result); } @@ -162,11 +168,11 @@ } })->with([[true, true], [false, true], [false, false]]); -it('does not requeue an unresolved timeout or crash and reconciles only explicit child outcomes', function ($status, $outcome, $success, $reset) { +it('does not requeue an unresolved timeout or crash and reconciles only explicit child outcomes', function ($outcome, $success, $reset, $status) { $GLOBALS['__test_cacti_log'] = []; plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 1]); - plugin_test_queue_db_result('db_fetch_row_prepared', $outcome); + plugin_test_queue_db_result('report_outcome', $outcome); plugin_test_queue_db_result('exec_with_timeout', $status); plugin_test_queue_db_result('exec_with_timeout_result', false); expect(flowview_reports_run(51))->toBe($success); @@ -179,9 +185,10 @@ } else { expect(implode(' ', $GLOBALS['__test_cacti_log']))->toContain('unresolved child outcome', '51'); } -})->with([[-1, ['status' => 'running'], false, false], [-1, false, false, false], [255, [], false, false], [-1, ['status' => 'complete'], true, false], [-1, ['status' => 'retry'], false, true]]); +})->with([[['status' => 'running'], false, false], [false, false, false], [[], false, false], [['status' => 'complete'], true, false], [['status' => 'retry'], false, true]])->with([0, 75, 1, -1, 255]); it('reconciles late complete and retry markers without losing queue ownership', function ($state, $reset, $claim, $success, $launches) { + plugin_test_queue_db_result('report_outcome', ['status' => 'complete']); plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'status' => $state, 'run_command' => 'fixture', 'run_timeout' => 1]); foreach ([$reset, $claim] as $value) { @@ -195,7 +202,7 @@ for ($i = 0; $i < 3; $i++) { plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); } - plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 51, 'status' => 'complete']]); + plugin_test_queue_db_result('completed_reports', [['id' => 51, 'source_id' => 7]]); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'status' => 'complete']); expect(flowview_reports_run_pending())->toBe(1); $exec = array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'exec_with_timeout'); @@ -212,9 +219,29 @@ }); it('uses the scheduler default when a report timeout is unset', function () { + plugin_test_queue_db_result('report_outcome', ['status' => 'complete']); plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 0]); expect(flowview_reports_run(51))->toBeTrue(); $exec = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'exec_with_timeout')); expect($exec[0]['params'][1])->toBe(300); }); + +it('continues executable work after a cleanup snapshot read fails', function () { + $GLOBALS['__test_cacti_log'] = []; + for ($i = 0; $i < 3; $i++) { plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); } + plugin_test_queue_db_result('completed_reports', false); + plugin_test_queue_db_result('db_fetch_assoc_prepared', [['id' => 51]]); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 1]); + plugin_test_queue_db_result('report_outcome', ['status' => 'complete']); + expect(flowview_reports_run_pending())->toBe(1); + expect(implode(' ', $GLOBALS['__test_cacti_log']))->toContain('Unable to read completed FlowView occurrences'); +}); + +it('retains its completed cleanup count when the executable snapshot read fails', function () { + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('completed_reports', [['id' => 51, 'source_id' => 7]]); + plugin_test_queue_db_result('db_fetch_assoc_prepared', false); + expect(flowview_reports_run_pending())->toBe(1); + expect(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'exec_with_timeout'))->toHaveCount(0); +}); diff --git a/tests/Unit/FilterScheduleRetryTest.php b/tests/Unit/FilterScheduleRetryTest.php index 8d23218..f746e66 100644 --- a/tests/Unit/FilterScheduleRetryTest.php +++ b/tests/Unit/FilterScheduleRetryTest.php @@ -29,6 +29,7 @@ plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'name' => 'Fixture', 'source_id' => 7, 'run_command' => 'fixture-command', 'run_timeout' => $status === 1 ? 0 : 60]); plugin_test_queue_db_result('exec_with_timeout', $startup ? null : $status); + plugin_test_queue_db_result('report_outcome', ['status' => $status === 0 ? 'complete' : 'retry']); plugin_test_queue_db_result('exec_with_timeout_result', ($startup || $warning) ? false : null); plugin_test_queue_db_result('db_execute_prepared', true); plugin_test_queue_db_result('db_execute_prepared', $reset); @@ -131,6 +132,7 @@ plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); plugin_test_queue_db_result('exec_with_timeout', $exit); + plugin_test_queue_db_result('report_outcome', ['status' => 'retry']); expect(flowview_reports_run(51))->toBeFalse(); expect(implode(' ', array_column($GLOBALS['__test_db_calls'], 'sql')))->not->toContain('DELETE FROM reports_queued'); } finally { diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 6f8960d..be6e165 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -152,6 +152,9 @@ function db_fetch_assoc($sql, $log = true, $conn = false) { if (!function_exists('db_fetch_assoc_prepared')) { function db_fetch_assoc_prepared($sql, $params = array(), $log = true, $conn = false) { $GLOBALS['__test_db_calls'][] = array('fn' => 'db_fetch_assoc_prepared', 'sql' => $sql, 'params' => $params, 'conn' => $conn); + if (str_starts_with($sql, 'SELECT id,source_id FROM reports_queued')) { + return plugin_test_db_result('completed_reports', []); + } return plugin_test_db_result('db_fetch_assoc_prepared', array()); } } @@ -166,6 +169,9 @@ function db_fetch_row($sql, $log = true, $conn = false) { if (!function_exists('db_fetch_row_prepared')) { function db_fetch_row_prepared($sql, $params = array(), $log = true, $conn = false) { $GLOBALS['__test_db_calls'][] = array('fn' => 'db_fetch_row_prepared', 'sql' => $sql, 'params' => $params, 'conn' => $conn); + if (str_starts_with($sql, 'SELECT status FROM reports_queued')) { + return plugin_test_db_result('report_outcome', []); + } return plugin_test_db_result('db_fetch_row_prepared', array()); } } diff --git a/tests/fixtures/completion-budget.php b/tests/fixtures/completion-budget.php new file mode 100644 index 0000000..e6ed513 --- /dev/null +++ b/tests/fixtures/completion-budget.php @@ -0,0 +1,46 @@ + $argv[2]]; +$db = new PDO('sqlite::memory:'); +$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); +$db->exec('CREATE TABLE reports_queued (id INTEGER PRIMARY KEY, source TEXT, source_id INTEGER, status TEXT, start_time TEXT, scheduled_time TEXT, run_command TEXT, run_timeout INTEGER)'); +for ($id = 1; $id <= 101; $id++) { + $stmt = $db->prepare("INSERT INTO reports_queued VALUES (?, 'flowview', ?, ?, '', '', 'fixture', 1)"); + $stmt->execute([$id, $id, $id === 101 ? 'pending' : 'complete']); +} +$attempts = []; +$deletes = []; +function db_fetch_row(...$args) { return ['Field' => 'enabled']; } +function db_fetch_assoc_prepared($sql, $params = [], ...$args) { + if (str_contains($sql, 'FROM processes')) { return []; } + $stmt = $GLOBALS['db']->prepare($sql); + $stmt->execute($params); + return $stmt->fetchAll(PDO::FETCH_ASSOC); +} +function db_fetch_row_prepared($sql, $params = [], ...$args) { return db_fetch_assoc_prepared($sql, $params)[0] ?? []; } +function db_execute_prepared($sql, $params = [], ...$args) { + if (str_starts_with($sql, 'DELETE FROM reports_queued')) { + $GLOBALS['deletes'][] = $params[0]; + if ($params[0] <= 100) { return false; } + } + $stmt = $GLOBALS['db']->prepare($sql); + $result = $stmt->execute($params); + $GLOBALS['affected'] = $stmt->rowCount(); + return $result; +} +function db_affected_rows(...$args) { return $GLOBALS['affected'] ?? 0; } +function exec_with_timeout($command, &$output, &$status, $timeout) { + preg_match('/--queue-id=(\d+)/', $command, $match); + $id = (int) $match[1]; + $GLOBALS['attempts'][] = $id; + db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=?', ['complete', $id]); + $status = 0; +} +function read_config_option(...$args) { return ''; } +function cacti_log(...$args) {} +require $argv[1] . '/includes/database.php'; +require $argv[1] . '/includes/functions.php'; +$done = flowview_reports_run_pending(1, 100); +$remaining = db_fetch_assoc_prepared('SELECT id,status FROM reports_queued ORDER BY id'); +echo json_encode(compact('done', 'attempts', 'deletes', 'remaining')); diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php index 91ced19..d0dbde0 100644 --- a/tests/fixtures/filter-lifecycle.php +++ b/tests/fixtures/filter-lifecycle.php @@ -4,6 +4,9 @@ $config = ['base_path' => dirname(__DIR__), 'poller_id' => 1]; $local_db_cnn_id = 'fixture'; $mode = getenv('FLOWVIEW_FIXTURE_MODE'); +if ($mode === 'connection-failure') { + define('FLOWVIEW_CONFIG', dirname(__DIR__) . '/connection.php'); +} $ready = in_array($mode, ['ready', 'late', 'child-disabled', 'report-read-failure', 'schedule-read-failure', 'filter-read-failure', 'parallel-timeout', 'timestamp-failure', 'master-busy'], true); $metadataReads = 0; $calls = []; @@ -113,3 +116,8 @@ function array_rekey($rows, $key, $value) { } function __esc($text, ...$args) { return html_escape($text); } + +function db_connect_real(...$args) { + $GLOBALS['calls'][] = ['connect']; + return false; +} diff --git a/tests/fixtures/filter-queue.php b/tests/fixtures/filter-queue.php index 1db5042..7ea7da9 100644 --- a/tests/fixtures/filter-queue.php +++ b/tests/fixtures/filter-queue.php @@ -37,6 +37,9 @@ function exec_with_timeout($command, &$output, &$status, $timeout) { $id = (int) $match[1]; $GLOBALS['attempts'][] = $id; $status = $id <= 2 || $id === 6 ? 75 : 0; + if ($status === 0) { + db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=?', ['complete', $id]); + } if ($status === 75) { db_execute_prepared('UPDATE reports_queued SET status=? WHERE id=?', ['retry', $id]); if ($id === 6) { diff --git a/tests/fixtures/report-stderr.php b/tests/fixtures/report-stderr.php index 3a70a05..2239451 100644 --- a/tests/fixtures/report-stderr.php +++ b/tests/fixtures/report-stderr.php @@ -9,6 +9,7 @@ $helperResult = exec_with_timeout($command, $output, $status, 5); plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'name' => 'Fixture', 'source_id' => 7, 'run_command' => $command, 'run_timeout' => 5]); +plugin_test_queue_db_result('report_outcome', ['status' => 'complete']); $success = $argv[4] === 'native' ? flowview_reports_run(51) : reports_run(51); $writes = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'db_execute_prepared')); echo json_encode(compact('helperResult', 'status', 'success', 'writes')); From 09755b607d0a114206c0caf5bc17a12ed24a70d6 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:52:30 +1030 Subject: [PATCH 27/29] Retry notification lookup failures before report delivery --- includes/functions-pre13.php | 11 +++++++-- includes/functions.php | 5 ++-- tests/Unit/FilterLifecycleRegressionTest.php | 24 ++++++++++++++++++ tests/Unit/FilterStatusTest.php | 5 +++- tests/fixtures/filter-lifecycle.php | 26 ++++++++++++++++---- 5 files changed, 61 insertions(+), 10 deletions(-) diff --git a/includes/functions-pre13.php b/includes/functions-pre13.php index 72ae7a8..a9d325b 100644 --- a/includes/functions-pre13.php +++ b/includes/functions-pre13.php @@ -44,14 +44,19 @@ * @param array $attachments Optional list of file attachments to include. * @param array $headers Optional extra email headers to include. * - * @return void + * @return bool False only when the queue lookup fails before any delivery. */ -function reports_log_and_notify($id, $start_time, $report_type, $source, $source_id, $subject, &$raw_data, &$oput_raw, &$oput_html, &$oput_text, $attachments = [], $headers = []): void { +function reports_log_and_notify($id, $start_time, $report_type, $source, $source_id, $subject, &$raw_data, &$oput_raw, &$oput_html, &$oput_text, $attachments = [], $headers = []): bool { $report = db_fetch_row_prepared('SELECT * FROM reports_queued WHERE id = ?', [$id]); + if ($report === false || !$report) { + cacti_log('FlowView notification queue lookup failed before delivery; retaining the occurrence for retry.', false, 'FLOWVIEW'); + return false; + } + if ($oput_text == null) { $oput_text = ''; } @@ -176,6 +181,8 @@ function reports_log_and_notify($id, $start_time, $report_type, $source, $source sql_save($save, 'reports_log'); } + + return true; } /** diff --git a/includes/functions.php b/includes/functions.php index c345d25..9be850e 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -2016,8 +2016,9 @@ function plugin_flowview_run_schedule($id, $report_id): bool { $headers['User-Agent'] = 'Cacti-FlowView-v' . $version; $headers['X-Priority'] = '1'; - reports_log_and_notify($report_id, $start_time, 'html', 'flowview', $id, $subject, $data['data'], $body, $body_html, $body_text, [], $headers); - return true; + // Native Cacti returns void; only the compatibility shim's explicit false + // certifies a pre-delivery failure that is safe to retry. + return reports_log_and_notify($report_id, $start_time, 'html', 'flowview', $id, $subject, $data['data'], $body, $body_html, $body_text, [], $headers) !== false; } /** diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index 99fb737..29fb65a 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -259,3 +259,27 @@ ->and($result['deletes'])->toHaveCount(201)->and($result['remaining'])->toHaveCount(100) ->and(array_unique(array_column($result['remaining'], 'status')))->toBe(['complete']); }); + + +it('retries only a pre-delivery notification lookup failure in the actual child', function ($native, $mode) { + $command = [PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/run_schedule.php', '--report-id=' . ($native ? 7 : 51)]; + if ($native) { $command[] = '--queue-id=51'; } + // The post-delivery fixture deliberately throws; suppress its expected fatal diagnostic. + if ($mode === 'notification-after-delivery') { + array_splice($command, 1, 0, ['-d', 'display_errors=0', '-d', 'log_errors=0']); + } + [$exit] = ($this->runFixture)($command, $mode); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + $writes = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'write')); + $actions = array_column($result['calls'], 0); + $queueReads = array_filter($result['calls'], fn ($c) => $c[0] === 'row' && str_contains($c[1], 'reports_queued')); + expect($queueReads)->toHaveCount(2); + if ($mode === 'notification-after-delivery') { + expect($exit)->toBe(255, json_encode($result))->and($actions)->toContain('delivery')->and($writes)->toBe([]); + } else { + expect($exit)->toBe(75)->and($actions)->not->toContain('delivery', 'save-log') + ->and($writes)->toHaveCount(1)->and($writes[0][2])->toBe(['retry', 51, 'flowview', 'running']); + } + $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); + expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0, $result['pid']]]]); +})->with([false, true])->with(['notification-read-failure', 'notification-missing', 'notification-after-delivery']); diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index d5a575b..696bd1d 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -87,7 +87,7 @@ expect($span)->toBe(['begin_now' => 200, 'end_now' => 2000]); }); -it('distinguishes prepared records, an empty period and a failed query', function ($data, $success) { +it('distinguishes prepared records, an empty period and a failed query', function ($data, $success, $notificationReport = 'valid') { plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); plugin_test_queue_db_result('db_fetch_row_prepared', ['query_id' => 3, 'title' => 'Fixture', 'format_file' => 'default']); plugin_test_queue_db_result('db_fetch_row_prepared', ['enabled' => 'on', 'timespan' => 1, 'name' => 'Fixture']); @@ -98,6 +98,7 @@ set_request_var('date2', date('Y-m-d H:i:s', 2000)); $key = get_flowview_session_key(3, 100, 2000); $_SESSION['sess_flowdata'][$key] = ['timeout' => time() + 60, 'data' => $data]; + plugin_test_queue_db_result('db_fetch_row_prepared', $notificationReport === 'valid' ? ['name' => 'Fixture', 'notification' => '', 'request_type' => 'scheduled', 'requested_by' => 'fixture', 'requested_id' => 1] : $notificationReport); $GLOBALS['__test_cacti_log'] = []; expect(plugin_flowview_run_schedule(7, 51))->toBe($success); $formatted = in_array('reports_load_format_file', array_column($GLOBALS['__test_db_calls'], 'fn'), true); @@ -109,6 +110,8 @@ } })->with([ [['table' => 'Fixture', 'data' => [['bytes' => 42]]], true], + [['table' => 'Fixture', 'data' => [['bytes' => 42]]], false, false], + [['table' => 'Fixture', 'data' => [['bytes' => 42]]], false, []], [['table' => 'Fixture', 'data' => []], true], [false, false], [['table' => 'Fixture', 'data' => false], false], diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php index d0dbde0..126aead 100644 --- a/tests/fixtures/filter-lifecycle.php +++ b/tests/fixtures/filter-lifecycle.php @@ -7,7 +7,7 @@ if ($mode === 'connection-failure') { define('FLOWVIEW_CONFIG', dirname(__DIR__) . '/connection.php'); } -$ready = in_array($mode, ['ready', 'late', 'child-disabled', 'report-read-failure', 'schedule-read-failure', 'filter-read-failure', 'parallel-timeout', 'timestamp-failure', 'master-busy'], true); +$ready = in_array($mode, ['ready', 'late', 'child-disabled', 'report-read-failure', 'schedule-read-failure', 'filter-read-failure', 'parallel-timeout', 'timestamp-failure', 'master-busy', 'notification-read-failure', 'notification-missing', 'notification-after-delivery'], true); $metadataReads = 0; $calls = []; const MESSAGE_LEVEL_INFO = 1; @@ -22,6 +22,7 @@ function cacti_sizeof($value) { return is_array($value) ? count($value) : 0; } function read_config_option($name) { + if (str_starts_with($GLOBALS['mode'], 'notification-') && $name === 'flowview_parallel_threads') { return 1; } return ['path_php_binary' => PHP_BINARY, 'reports_concurrent' => 1, 'flowview_parallel_threads' => 2, 'flowview_partition' => 1, 'flowview_parallel_time_to_live' => 21600, 'flowview_parallel_runlimit' => 0][$name] ?? ''; } function cacti_log(...$args) { @@ -81,12 +82,18 @@ function db_fetch_row_prepared($sql, $params = [], ...$args) { return false; } if (str_contains($sql, 'reports_queued')) { + $reads = array_filter($GLOBALS['calls'], fn ($c) => $c[0] === 'row' && str_contains($c[1], 'reports_queued')); + if (count($reads) > 1 && $GLOBALS['mode'] === 'notification-read-failure') { return false; } + if (count($reads) > 1 && $GLOBALS['mode'] === 'notification-missing') { return []; } + if ($GLOBALS['mode'] === 'notification-after-delivery') { + return ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'name' => 'Fixture', 'notification' => '{"email":{"to_email":"fixture@example.invalid"}}']; + } return ['id' => 51, 'source' => 'flowview', 'source_id' => 7]; } if (str_contains($sql, 'plugin_flowview_schedules')) { return ['id' => 7, 'query_id' => 3, 'title' => 'Fixture', 'format_file' => 'default']; } - return $GLOBALS['mode'] === 'parallel-timeout' + return ($GLOBALS['mode'] === 'parallel-timeout' || str_starts_with($GLOBALS['mode'], 'notification-')) ? ['id' => 3, 'name' => 'Fixture', 'enabled' => 'on', 'timespan' => 1, 'statistics' => 9, 'printed' => 0, 'sortfield' => 'bytes', 'cutofflines' => 100] : ['enabled' => '']; } @@ -99,14 +106,19 @@ function db_fetch_assoc($sql, ...$args) { return str_contains($sql, 'information_schema') ? [['table' => 'plugin_flowview_raw_202400000']] : []; } function db_fetch_assoc_prepared(...$args) { - return []; + return str_starts_with($GLOBALS['mode'], 'notification-') ? [['bytes' => 42]] : []; } function get_timespan(&$span, ...$args) { $span = ['begin_now' => 1704067200, 'end_now' => 1704070800]; + if (str_starts_with($GLOBALS['mode'], 'notification-')) { + $span = ['begin_now' => 0, 'end_now' => 2000]; + $key = get_flowview_session_key(3, 100, 2000); + $_SESSION['sess_flowdata'][$key] = ['timeout' => time() + 60, 'data' => ['table' => 'Fixture', 'data' => [['bytes' => 42]]]]; + } } function read_user_setting(...$args) { return ''; } -function isset_request_var(...$args) { return false; } -function get_request_var(...$args) { return null; } +function isset_request_var(...$args) { return str_starts_with($GLOBALS['mode'], 'notification-') && in_array($args[0], ['timespan', 'query', 'date1', 'date2'], true); } +function get_request_var(...$args) { return ['timespan' => 'custom', 'query' => 3, 'date1' => date('Y-m-d H:i:s', 100), 'date2' => date('Y-m-d H:i:s', 2000)][$args[0]] ?? null; } function get_nfilter_request_var(...$args) { return null; } function get_filter_request_var(...$args) { return null; } function cacti_session_close() {} @@ -121,3 +133,7 @@ function db_connect_real(...$args) { $GLOBALS['calls'][] = ['connect']; return false; } + +function reports_load_format_file($format, &$output, &$tag, &$theme) { $output = ''; $tag = true; return true; } +function mailer(...$args) { $GLOBALS['calls'][] = ['delivery']; throw new RuntimeException('Ambiguous delivery failure'); } +function sql_save(...$args) { $GLOBALS['calls'][] = ['save-log']; return 1; } From a3bc07827dabc8504e962c2709f4e4bca3ca20ff Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Wed, 21 Oct 2026 00:04:42 +1030 Subject: [PATCH 28/29] Preserve uncertain notification outcomes across Cacti versions --- README.md | 2 + includes/functions-pre13.php | 162 +------------------ includes/functions.php | 11 +- includes/report_delivery.php | 80 +++++++++ phpunit.xml | 1 + run_schedule.php | 7 +- tests/Unit/FilterLifecycleRegressionTest.php | 64 +++++++- tests/Unit/FilterStatusTest.php | 1 + tests/Unit/ReportDeliveryTest.php | 70 ++++++++ tests/bootstrap-unit.php | 11 +- tests/fixtures/filter-lifecycle.php | 23 ++- 11 files changed, 259 insertions(+), 173 deletions(-) create mode 100644 includes/report_delivery.php create mode 100644 tests/Unit/ReportDeliveryTest.php diff --git a/README.md b/README.md index 6432e86..1ddac5b 100644 --- a/README.md +++ b/README.md @@ -243,3 +243,5 @@ Do not reset an unresolved row merely because its process has disappeared. Completion cleanup and executable reports use separate bounded snapshots, so persistent cleanup failures cannot consume the pending-work budget. Cleanup still runs first and does not require a free execution slot. + +Scheduled FlowView notifications use one observable delivery helper on Cacti 1.2 and 1.3. A failed queue lookup before delivery requests retry. A returned mailer error, invalid notification configuration, or failed report-log write holds the occurrence for manual reconciliation without advancing schedule timestamps. This includes partial delivery to multiple notification targets; it is never automatically resent. diff --git a/includes/functions-pre13.php b/includes/functions-pre13.php index a9d325b..d5f3ded 100644 --- a/includes/functions-pre13.php +++ b/includes/functions-pre13.php @@ -24,165 +24,11 @@ */ /** - * Pre-1.3 Cacti compatibility shim for reports_log_and_notify(): - * records a completed queued report's result and notification - * settings into the reports_log table and sends its notification - * (attachments/email) via the report's stored delivery settings. - * Called from this plugin's report-running code on Cacti versions - * lacking the native reports_log_and_notify() function. - * - * @param int $id The reports_queued id being completed. - * @param float $start_time The Unix timestamp the run started at. - * @param string $report_type The rendered report format (e.g. 'html'). - * @param string $source The plugin/source name generating the report (e.g. 'flowview'). - * @param int $source_id The source's own report/schedule id. - * @param string $subject The notification email subject. - * @param array $raw_data Reference, the report's raw result data. - * @param mixed $oput_raw Reference, the raw rendered output. - * @param string $oput_html Reference, the HTML-rendered output body. - * @param string|null $oput_text Reference, the plain-text output body (defaulted to '' if null). - * @param array $attachments Optional list of file attachments to include. - * @param array $headers Optional extra email headers to include. - * - * @return bool False only when the queue lookup fails before any delivery. + * Compatibility name for callers on Cacti versions without native reporting. + * False means safe to retry; null means delivery requires manual reconciliation. */ -function reports_log_and_notify($id, $start_time, $report_type, $source, $source_id, $subject, &$raw_data, &$oput_raw, &$oput_html, &$oput_text, $attachments = [], $headers = []): bool { - $report = db_fetch_row_prepared('SELECT * - FROM reports_queued - WHERE id = ?', - [$id]); - - if ($report === false || !$report) { - cacti_log('FlowView notification queue lookup failed before delivery; retaining the occurrence for retry.', false, 'FLOWVIEW'); - return false; - } - - if ($oput_text == null) { - $oput_text = ''; - } - - $fromemail = read_config_option('settings_from_email'); - if ($fromemail == '') { - $fromemail = 'cacti@cacti.net'; - } - - $fromname = read_config_option('settings_from_name'); - if ($fromname == '') { - $fromname = __('Cacti %s', ucfirst($source), 'flowview'); - } - - $from[0] = $fromemail; - $from[1] = $fromname; - - if (cacti_sizeof($report)) { - if ($report['notification'] != '') { - $notifications = json_decode($report['notification'], true); - - foreach($notifications as $type => $data) { - switch($type) { - case 'email': - if (!isset($data['to_email'])) { - cacti_log(sprintf("WARNING: Email Report '%s' not sent! Missing 'to_email' attribute in request", $report['name']), false, 'REPORTS'); - break; - } else { - $to_email = $data['to_email']; - } - - if (isset($data['cc_email'])) { - $cc_email = $data['cc_email']; - } else { - $cc_email = ''; - } - - if (isset($data['bcc_email'])) { - $bcc_email = $data['bcc_email']; - } else { - $bcc_email = ''; - } - - if (isset($data['reply_to'])) { - $reply_to = $data['reply_to']; - } else { - $reply_to = ''; - } - - mailer($from, $to_email, $cc_email, $bcc_email, $reply_to, $subject, $oput_html, $oput_text, $attachments, $headers); - - break; - case 'notification_list': - if (!isset($data['id'])) { - cacti_log(sprintf("WARNING: Email Report '%s' not sent! Missing notification list 'id' attribute in request", $report['name']), false, 'REPORTS'); - break; - } else { - $list = db_fetch_row_prepared('SELECT * - FROM plugin_notify_list - WHERE id = ?', - [$data['id']]); - - if (cacti_sizeof($list)) { - /* process the format file */ - $report_tag = ''; - $theme = 'modern'; - $output_html = ''; - $format = $list['format_file'] != '' ? $list['format_file']:'default'; - - $format_ok = reports_load_format_file($format, $output, $report_tag, $theme); - - flowview_debug('Format File Loaded, Format is ' . ($format_ok ? 'Ok':'Not Ok') . ', Report Tag is ' . $report_tag); - - if ($format_ok) { - if ($report_tag) { - $oput_html = str_replace('', $oput_raw, $output); - } else { - $oput_html = $output . PHP_EOL . $oput_raw; - } - } else { - $oput_html = $oput_raw; - } - - $to_email = $list['emails']; - $cc_emails = isset($list['cc_emails']) ? $list['cc_emails']:''; - $bcc_emails = $list['bcc_emails']; - $reply_to = isset($list['reply_to']) ? $list['reply_to']:''; - - mailer($from, $to_email, $cc_emails, $bcc_emails, $reply_to, $subject, $oput_html, $oput_text, $attachments, $headers); - } else { - cacti_log(sprintf("WARNING: Email Report '%s' not sent! Unable to locate notification list '%s'", $report['name'], $id), false, 'REPORTS'); - } - } - - break; - default: - cacti_log(sprintf("WARNING: Email Report '%s' not sent! Unknown notification type '%s' attribute in request", $report['name'], $type), false, 'REPORTS'); - break; - } - } - } - - $end_time = microtime(true); - - $save = []; - - $save['id'] = 0; - $save['name'] = $report['name']; - $save['source'] = $source; - $save['source_id'] = $source_id; - $save['report_output_type'] = $report_type; - $save['report_raw_data'] = json_encode($raw_data); - $save['report_raw_output'] = $oput_raw; - $save['report_html_output'] = $oput_html; - $save['report_txt_output'] = $oput_text; - $save['send_type'] = $report['request_type']; - $save['send_time'] = date('Y-m-d H:i:s'); - $save['run_time'] = $end_time - $start_time; - $save['sent_by'] = $report['requested_by']; - $save['sent_id'] = $report['requested_id']; - $save['notification'] = $report['notification']; - - sql_save($save, 'reports_log'); - } - - return true; +function reports_log_and_notify($id, $start_time, $report_type, $source, $source_id, $subject, &$raw_data, &$oput_raw, &$oput_html, &$oput_text, $attachments = [], $headers = []): ?bool { + return flowview_reports_log_and_notify($id, $start_time, $report_type, $source, $source_id, $subject, $raw_data, $oput_raw, $oput_html, $oput_text, $attachments, $headers); } /** diff --git a/includes/functions.php b/includes/functions.php index 9be850e..5f427c3 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -23,6 +23,8 @@ +-------------------------------------------------------------------------+ */ +require_once(__DIR__ . '/report_delivery.php'); + // reports_run()/reports_queue()/reports_log_and_notify() became native in // Cacti 1.3. Whether core will declare them is fixed by the Cacti version at // bootstrap, so gate the pre-1.3 shims on that rather than on function_exists(): @@ -1916,9 +1918,9 @@ function get_port_name($port_num, int $port_proto = 6) { * @param mixed $id * @param mixed $report_id * - * @return bool False when schema readiness or query execution requires retry; true otherwise. + * @return bool|null False permits retry; null holds uncertain delivery for repair; true completes. */ -function plugin_flowview_run_schedule($id, $report_id): bool { +function plugin_flowview_run_schedule($id, $report_id): ?bool { global $config; if (!flowview_filter_status_ready()) { @@ -2016,9 +2018,8 @@ function plugin_flowview_run_schedule($id, $report_id): bool { $headers['User-Agent'] = 'Cacti-FlowView-v' . $version; $headers['X-Priority'] = '1'; - // Native Cacti returns void; only the compatibility shim's explicit false - // certifies a pre-delivery failure that is safe to retry. - return reports_log_and_notify($report_id, $start_time, 'html', 'flowview', $id, $subject, $data['data'], $body, $body_html, $body_text, [], $headers) !== false; + // Core's void notification helper cannot expose lookup or delivery failures. + return flowview_reports_log_and_notify($report_id, $start_time, 'html', 'flowview', $id, $subject, $data['data'], $body, $body_html, $body_text, [], $headers); } /** diff --git a/includes/report_delivery.php b/includes/report_delivery.php new file mode 100644 index 0000000..a757bb8 --- /dev/null +++ b/includes/report_delivery.php @@ -0,0 +1,80 @@ + read_config_option('settings_from_email') ?: 'cacti@cacti.net', + 'name' => read_config_option('settings_from_name') ?: 'Cacti ' . ucfirst($source) + ]; + $oput_text ??= ''; + foreach ($notifications as $type => $data) { + if (!is_array($data)) { + return flowview_delivery_unresolved($id); + } + $html = $oput_html; + if ($type === 'email' && !empty($data['to_email'])) { + $to = $data['to_email']; + $cc = $data['cc_email'] ?? ''; + $bcc = $data['bcc_email'] ?? ''; + $reply = $data['reply_to'] ?? ''; + } elseif ($type === 'notification_list' && !empty($data['id'])) { + $list = db_fetch_row_prepared('SELECT * FROM plugin_notification_lists WHERE id = ?', [$data['id']]); + if (!$list || empty($list['emails'])) { + return flowview_delivery_unresolved($id); + } + $output = ''; + $tag = false; + $theme = 'modern'; + if (reports_load_format_file($list['format_file'] ?: 'default', $output, $tag, $theme)) { + $html = $tag ? str_replace('', $oput_raw, $output) : $output . PHP_EOL . $oput_raw; + } else { + $html = $oput_raw; + } + $to = $list['emails']; + $cc = $list['cc_emails'] ?? ''; + $bcc = $list['bcc_emails'] ?? ''; + $reply = $list['reply_to'] ?? ($data['reply_to'] ?? ''); + } else { + return flowview_delivery_unresolved($id); + } + // Cacti's mailer returns an empty error string on success. An error + // can follow partial delivery, so it must never produce a safe retry. + $error = mailer($data['from'] ?? $from, $to, $cc, $bcc, $reply, $subject, $html, $oput_text, $attachments, $headers); + if ($error !== '') { + return flowview_delivery_unresolved($id); + } + } + $save = [ + 'id' => 0, 'name' => $report['name'], 'source' => $source, 'source_id' => $source_id, + 'report_output_type' => $report_type, 'report_raw_data' => json_encode($raw_data), + 'report_raw_output' => $oput_raw, 'report_html_output' => $oput_html, 'report_txt_output' => $oput_text, + 'send_type' => $report['request_type'], 'send_time' => date('Y-m-d H:i:s'), + 'run_time' => microtime(true) - $start_time, 'sent_by' => $report['requested_by'], + 'sent_id' => $report['requested_id'], 'notification' => $report['notification'] + ]; + if (!sql_save($save, 'reports_log')) { + return flowview_delivery_unresolved($id); + } + return true; +} + +function flowview_delivery_unresolved($id): ?bool { + cacti_log('FlowView occurrence ' . $id . ' notification or report-log failure; delivery may be partial. Hold for repair and verify delivery before resending.', false, 'FLOWVIEW'); + return null; +} diff --git a/phpunit.xml b/phpunit.xml index b2306a5..6139bae 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -39,6 +39,7 @@ Net/DNS2/Updater.php includes/functions.php includes/functions-pre13.php + includes/report_delivery.php includes/database.php includes/filter_upgrade.php diff --git a/run_schedule.php b/run_schedule.php index 6cf314d..ce8407e 100644 --- a/run_schedule.php +++ b/run_schedule.php @@ -188,7 +188,12 @@ } if (cacti_sizeof($schedule)) { - if (!plugin_flowview_run_schedule($id, $report_id)) { + $delivery = plugin_flowview_run_schedule($id, $report_id); + if ($delivery === null) { + cacti_log('FlowView occurrence ' . $report_id . ' has an unresolved notification or log failure; verify delivery before repairing the queue.', false, 'FLOWVIEW'); + exit(76); + } + if ($delivery === false) { // A deferred or failed preparation must preserve the occurrence // and schedule timestamps just like the initial readiness check. $outcome = 'retry'; diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index 29fb65a..d32a321 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -19,8 +19,21 @@ foreach (['poller.php', 'time.php', 'timespan_settings.php'] as $file) { file_put_contents($this->fixtureRoot . '/lib/' . $file, 'runFixture = function ($command, $mode = 'pending') { + $this->runFixture = function ($command, $mode = 'pending', $nativeCore = false) { $env = getenv(); + if ($nativeCore) { + global $config; + $source = getenv('FLOWVIEW_NATIVE_REPORTS_SOURCE') ?: $config['base_path'] . '/lib/reports.php'; + $text = file_get_contents($source); + $start = strpos($text, 'function reports_log_and_notify('); + if ($start === false) { $this->markTestSkipped('Native Cacti notification helper not available'); } + // Load the actual official helper in isolation, without core's unrelated declarations. + $end = strpos($text, "\nfunction reports_queue(", $start); + expect($end)->not->toBeFalse(); + $env['FLOWVIEW_FIXTURE_NATIVE_HELPER'] = $this->fixtureRoot . '/native-notify.php'; + file_put_contents($env['FLOWVIEW_FIXTURE_NATIVE_HELPER'], 'fixtureRoot . '/result.json'; $process = proc_open($command, [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes, $this->fixtureRoot . '/plugins/flowview', $env); @@ -283,3 +296,52 @@ $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0, $result['pid']]]]); })->with([false, true])->with(['notification-read-failure', 'notification-missing', 'notification-after-delivery']); + + +it('holds returned notification errors instead of retrying or completing', function ($native, $mode, $nativeCore) { + $command = [PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/run_schedule.php', '--report-id=' . ($native ? 7 : 51)]; + if ($native) { $command[] = '--queue-id=51'; } + [$exit] = ($this->runFixture)($command, $mode, $nativeCore); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + $writes = array_filter($result['calls'], fn ($c) => $c[0] === 'write'); + expect($exit)->toBe(76)->and($writes)->toBe([]) + ->and(array_column($result['calls'], 0))->toContain('delivery'); + expect($result['native_helper_loaded'])->toBe($nativeCore); + // The parent must retain the held child outcome, regardless of launch form. + plugin_test_reset(); + plugin_test_queue_db_result('db_fetch_row', ['Field' => 'enabled']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'run_command' => 'fixture', 'run_timeout' => 60]); + plugin_test_queue_db_result('exec_with_timeout', $exit); + plugin_test_queue_db_result('report_outcome', ['status' => 'running']); + global $config; + $oldBase = $config['base_path']; + $config['base_path'] = $this->fixtureRoot; + try { + expect($native ? flowview_reports_run(51) : reports_run(51))->toBeFalse(); + } finally { + $config['base_path'] = $oldBase; + } + + $mutations = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => str_starts_with($c['sql'], 'DELETE FROM reports_queued') || ($c['params'][0] ?? '') === 'pending')); + expect($mutations)->toBe([]); + + $released = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'unregister')); + expect($released)->toBe([['unregister', ['flowsched', 'child_7', 0, $result['pid']]]]); +})->with([false, true])->with(['notification-mail-error', 'notification-list-error', 'notification-log-error'])->with([false, true]); + +it('exposes lookup outcomes with the official native helper loaded', function ($mode) { + $command = [PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/run_schedule.php', '--report-id=7', '--queue-id=51']; + [$exit] = ($this->runFixture)($command, $mode, true); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + $queueReads = array_filter($result['calls'], fn ($c) => $c[0] === 'row' && str_contains($c[1], 'reports_queued')); + $writes = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'write')); + expect($queueReads)->toHaveCount(2)->and($result['native_helper_loaded'])->toBeTrue(); + if ($mode === 'notification-success') { + expect($exit)->toBe(0)->and($writes)->toHaveCount(2) + ->and(end($writes)[2])->toBe(['complete', 51, 'flowview', 'running']); + } else { + expect($exit)->toBe(75)->and($writes)->toHaveCount(1) + ->and($writes[0][2])->toBe(['retry', 51, 'flowview', 'running']) + ->and(array_column($result['calls'], 0))->not->toContain('delivery', 'save-log'); + } +})->with(['notification-read-failure', 'notification-missing', 'notification-success']); diff --git a/tests/Unit/FilterStatusTest.php b/tests/Unit/FilterStatusTest.php index 696bd1d..6752d17 100644 --- a/tests/Unit/FilterStatusTest.php +++ b/tests/Unit/FilterStatusTest.php @@ -99,6 +99,7 @@ $key = get_flowview_session_key(3, 100, 2000); $_SESSION['sess_flowdata'][$key] = ['timeout' => time() + 60, 'data' => $data]; plugin_test_queue_db_result('db_fetch_row_prepared', $notificationReport === 'valid' ? ['name' => 'Fixture', 'notification' => '', 'request_type' => 'scheduled', 'requested_by' => 'fixture', 'requested_id' => 1] : $notificationReport); + plugin_test_queue_db_result('sql_save', 1); $GLOBALS['__test_cacti_log'] = []; expect(plugin_flowview_run_schedule(7, 51))->toBe($success); $formatted = in_array('reports_load_format_file', array_column($GLOBALS['__test_db_calls'], 'fn'), true); diff --git a/tests/Unit/ReportDeliveryTest.php b/tests/Unit/ReportDeliveryTest.php new file mode 100644 index 0000000..07a802c --- /dev/null +++ b/tests/Unit/ReportDeliveryTest.php @@ -0,0 +1,70 @@ + 'Fixture', 'notification' => $payload, 'request_type' => 'scheduled', + 'requested_by' => 'fixture', 'requested_id' => 1 + ]); + plugin_test_queue_db_result('db_fetch_row_prepared', $list); + plugin_test_queue_db_result('mailer', $mailError); + plugin_test_queue_db_result('sql_save', $saved); + plugin_test_queue_db_result('delivery_format_ok', $format); + plugin_test_queue_db_result('delivery_format_tag', $tag); + $data = [['bytes' => 42]]; + $raw = '

Fixture

'; + $html = 'Fixture'; + $text = null; + expect(flowview_reports_log_and_notify(51, microtime(true), 'html', 'flowview', 7, 'Fixture', $data, $raw, $html, $text))->toBe($expected); + $mail = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'mailer')); + if ($expected === true) { + expect($GLOBALS['__test_saved_rows'])->toHaveCount(1) + ->and($GLOBALS['__test_saved_rows'][0]['table'])->toBe('reports_log'); + } elseif ($mailError !== '' || $saved === false) { + expect($mail)->toHaveCount(1); + } else { + expect($mail)->toBe([]); + } +})->with([ + ['', [], '', 1, true], + ['{invalid', [], '', 1, null], + ['null', [], '', 1, null], + ['{"email":false}', [], '', 1, null], + ['{"email":{}}', [], '', 1, null], + ['{"unknown":{}}', [], '', 1, null], + ['{"notification_list":{}}', [], '', 1, null], + ['{"notification_list":{"id":1}}', false, '', 1, null], + ['{"notification_list":{"id":1}}', [], '', 1, null], + ['{"notification_list":{"id":1}}', ['emails' => ''], '', 1, null], + ['{"email":{"to_email":"to@example.invalid"}}', [], '', 1, true], + ['{"email":{"to_email":"to@example.invalid","cc_email":"cc@example.invalid","bcc_email":"bcc@example.invalid","reply_to":"reply@example.invalid","from":{"email":"from@example.invalid","name":"Fixture"}}}', [], '', 1, true], + ['{"email":{"to_email":"to@example.invalid"}}', [], 'SMTP failed', 1, null], + ['{"email":{"to_email":"to@example.invalid"}}', [], '', false, null], + ['{"notification_list":{"id":1}}', ['emails' => 'to@example.invalid', 'format_file' => ''], '', 1, true], + ['{"notification_list":{"id":1,"reply_to":"fallback@example.invalid"}}', ['emails' => 'to@example.invalid', 'format_file' => 'default', 'cc_emails' => 'cc@example.invalid', 'bcc_emails' => 'bcc@example.invalid', 'reply_to' => 'reply@example.invalid'], '', 1, true, true, false], + ['{"notification_list":{"id":1}}', ['emails' => 'to@example.invalid', 'format_file' => 'default'], '', 1, true, false], + ['{"notification_list":{"id":1}}', ['emails' => 'to@example.invalid', 'format_file' => 'default'], 'SMTP failed', 1, null], +]); + +it('keeps a partial multi-notification delivery unresolved without writing a success log', function () { + plugin_test_queue_db_result('db_fetch_row_prepared', ['notification' => '{"email":{"to_email":"to@example.invalid"},"notification_list":{"id":1}}']); + plugin_test_queue_db_result('db_fetch_row_prepared', ['emails' => 'list@example.invalid', 'format_file' => 'default']); + plugin_test_queue_db_result('mailer', ''); + plugin_test_queue_db_result('mailer', 'SMTP failed after first notification'); + $data = []; $raw = ''; $html = ''; $text = ''; + expect(flowview_reports_log_and_notify(51, microtime(true), 'html', 'flowview', 7, 'Fixture', $data, $raw, $html, $text))->toBeNull(); + $mail = array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'mailer'); + expect($mail)->toHaveCount(2)->and($GLOBALS['__test_saved_rows'])->toBe([]); +}); + + +it('preserves the compatibility entrypoint outcome without hiding lookup failures', function ($report, $expected) { + plugin_test_queue_db_result('db_fetch_row_prepared', $report); + plugin_test_queue_db_result('sql_save', 1); + $data = []; $raw = ''; $html = ''; $text = ''; + expect(reports_log_and_notify(51, microtime(true), 'html', 'flowview', 7, 'Fixture', $data, $raw, $html, $text))->toBe($expected); +})->with([ + [false, false], [[], false], + [['notification' => '', 'name' => 'Fixture', 'request_type' => 'scheduled', 'requested_by' => 'fixture', 'requested_id' => 1], true] +]); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index be6e165..77bb8d8 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -535,8 +535,8 @@ function register_process_start($type, $name, $id = 0, $timeout = 300) { function reports_load_format_file($format, &$output, &$report_tag, &$theme) { $GLOBALS['__test_db_calls'][] = ['fn' => 'reports_load_format_file', 'sql' => '', 'params' => [$format]]; $output = ''; - $report_tag = true; - return true; + $report_tag = plugin_test_db_result('delivery_format_tag', true); + return plugin_test_db_result('delivery_format_ok', true); } } @@ -566,3 +566,10 @@ function db_debug($message) { $GLOBALS['__test_db_calls'][] = ['fn' => 'db_debug', 'sql' => '', 'params' => [$message]]; } } + +if (!function_exists('mailer')) { + function mailer(...$args) { + $GLOBALS['__test_db_calls'][] = ['fn' => 'mailer', 'sql' => '', 'params' => $args]; + return plugin_test_db_result('mailer', ''); + } +} diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php index 126aead..de86429 100644 --- a/tests/fixtures/filter-lifecycle.php +++ b/tests/fixtures/filter-lifecycle.php @@ -7,7 +7,7 @@ if ($mode === 'connection-failure') { define('FLOWVIEW_CONFIG', dirname(__DIR__) . '/connection.php'); } -$ready = in_array($mode, ['ready', 'late', 'child-disabled', 'report-read-failure', 'schedule-read-failure', 'filter-read-failure', 'parallel-timeout', 'timestamp-failure', 'master-busy', 'notification-read-failure', 'notification-missing', 'notification-after-delivery'], true); +$ready = in_array($mode, ['ready', 'late', 'child-disabled', 'report-read-failure', 'schedule-read-failure', 'filter-read-failure', 'parallel-timeout', 'timestamp-failure', 'master-busy', 'notification-read-failure', 'notification-missing', 'notification-after-delivery', 'notification-mail-error', 'notification-list-error', 'notification-log-error', 'notification-success'], true); $metadataReads = 0; $calls = []; const MESSAGE_LEVEL_INFO = 1; @@ -71,7 +71,7 @@ function set_default_action() { // Capture state after shutdown callbacks registered by the actual entrypoint. register_shutdown_function(function () { register_shutdown_function(function () { - file_put_contents(getenv('FLOWVIEW_FIXTURE_RESULT'), json_encode(['calls' => $GLOBALS['calls'], 'status' => http_response_code(), 'repair_loaded' => function_exists('flowview_filter_status_upgrade'), 'pid' => getmypid()])); + file_put_contents(getenv('FLOWVIEW_FIXTURE_RESULT'), json_encode(['calls' => $GLOBALS['calls'], 'status' => http_response_code(), 'repair_loaded' => function_exists('flowview_filter_status_upgrade'), 'pid' => getmypid(), 'native_helper_loaded' => getenv('FLOWVIEW_FIXTURE_NATIVE_HELPER') ? realpath((new ReflectionFunction('reports_log_and_notify'))->getFileName()) === realpath(getenv('FLOWVIEW_FIXTURE_NATIVE_HELPER')) : false])); }); }); function db_fetch_row_prepared($sql, $params = [], ...$args) { @@ -85,11 +85,12 @@ function db_fetch_row_prepared($sql, $params = [], ...$args) { $reads = array_filter($GLOBALS['calls'], fn ($c) => $c[0] === 'row' && str_contains($c[1], 'reports_queued')); if (count($reads) > 1 && $GLOBALS['mode'] === 'notification-read-failure') { return false; } if (count($reads) > 1 && $GLOBALS['mode'] === 'notification-missing') { return []; } - if ($GLOBALS['mode'] === 'notification-after-delivery') { - return ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'name' => 'Fixture', 'notification' => '{"email":{"to_email":"fixture@example.invalid"}}']; + if (in_array($GLOBALS['mode'], ['notification-after-delivery', 'notification-mail-error', 'notification-log-error', 'notification-success', 'notification-list-error'], true)) { + return ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'name' => 'Fixture', 'notification' => $GLOBALS['mode'] === 'notification-list-error' ? '{"notification_list":{"id":1}}' : '{"email":{"to_email":"fixture@example.invalid"}}', 'request_type' => 'scheduled', 'requested_by' => 'fixture', 'requested_id' => 1]; } return ['id' => 51, 'source' => 'flowview', 'source_id' => 7]; } + if (str_contains($sql, 'plugin_notification_lists')) { return ['emails' => 'list@example.invalid', 'format_file' => 'default']; } if (str_contains($sql, 'plugin_flowview_schedules')) { return ['id' => 7, 'query_id' => 3, 'title' => 'Fixture', 'format_file' => 'default']; } @@ -135,5 +136,15 @@ function db_connect_real(...$args) { } function reports_load_format_file($format, &$output, &$tag, &$theme) { $output = ''; $tag = true; return true; } -function mailer(...$args) { $GLOBALS['calls'][] = ['delivery']; throw new RuntimeException('Ambiguous delivery failure'); } -function sql_save(...$args) { $GLOBALS['calls'][] = ['save-log']; return 1; } +function mailer(...$args) { + $GLOBALS['calls'][] = ['delivery']; + if ($GLOBALS['mode'] === 'notification-after-delivery') { throw new RuntimeException('Ambiguous delivery failure'); } + return in_array($GLOBALS['mode'], ['notification-mail-error', 'notification-list-error'], true) ? 'SMTP delivery failed' : ''; +} +function sql_save(...$args) { $GLOBALS['calls'][] = ['save-log']; return $GLOBALS['mode'] === 'notification-log-error' ? false : 1; } + +if (getenv('FLOWVIEW_FIXTURE_NATIVE_HELPER')) { + define('CACTI_VERSION', '1.3.0'); + function cacti_version_compare($a, $b, $op) { return version_compare($a, $b, $op); } + require getenv('FLOWVIEW_FIXTURE_NATIVE_HELPER'); +} From 59d8b6b34b4175f75eab5cc4925a1f9f7b5c084e Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Wed, 21 Oct 2026 00:27:37 +1030 Subject: [PATCH 29/29] Accept CC-only and BCC-only report recipients --- includes/report_delivery.php | 10 ++++---- tests/Unit/FilterLifecycleRegressionTest.php | 19 +++++++++++++- tests/Unit/ReportDeliveryTest.php | 26 ++++++++++++++++++++ tests/fixtures/filter-lifecycle.php | 8 +++--- 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/includes/report_delivery.php b/includes/report_delivery.php index a757bb8..743f811 100644 --- a/includes/report_delivery.php +++ b/includes/report_delivery.php @@ -28,25 +28,25 @@ function flowview_reports_log_and_notify($id, $start_time, $report_type, $source return flowview_delivery_unresolved($id); } $html = $oput_html; - if ($type === 'email' && !empty($data['to_email'])) { - $to = $data['to_email']; + if ($type === 'email' && (!empty($data['to_email']) || !empty($data['cc_email']) || !empty($data['bcc_email']))) { + $to = $data['to_email'] ?? ''; $cc = $data['cc_email'] ?? ''; $bcc = $data['bcc_email'] ?? ''; $reply = $data['reply_to'] ?? ''; } elseif ($type === 'notification_list' && !empty($data['id'])) { $list = db_fetch_row_prepared('SELECT * FROM plugin_notification_lists WHERE id = ?', [$data['id']]); - if (!$list || empty($list['emails'])) { + if (!$list || (empty($list['emails']) && empty($list['cc_emails']) && empty($list['bcc_emails']))) { return flowview_delivery_unresolved($id); } $output = ''; $tag = false; $theme = 'modern'; - if (reports_load_format_file($list['format_file'] ?: 'default', $output, $tag, $theme)) { + if (reports_load_format_file(($list['format_file'] ?? '') ?: 'default', $output, $tag, $theme)) { $html = $tag ? str_replace('', $oput_raw, $output) : $output . PHP_EOL . $oput_raw; } else { $html = $oput_raw; } - $to = $list['emails']; + $to = $list['emails'] ?? ''; $cc = $list['cc_emails'] ?? ''; $bcc = $list['bcc_emails'] ?? ''; $reply = $list['reply_to'] ?? ($data['reply_to'] ?? ''); diff --git a/tests/Unit/FilterLifecycleRegressionTest.php b/tests/Unit/FilterLifecycleRegressionTest.php index d32a321..dcc5208 100644 --- a/tests/Unit/FilterLifecycleRegressionTest.php +++ b/tests/Unit/FilterLifecycleRegressionTest.php @@ -19,7 +19,7 @@ foreach (['poller.php', 'time.php', 'timespan_settings.php'] as $file) { file_put_contents($this->fixtureRoot . '/lib/' . $file, 'runFixture = function ($command, $mode = 'pending', $nativeCore = false) { + $this->runFixture = function ($command, $mode = 'pending', $nativeCore = false, $recipients = null) { $env = getenv(); if ($nativeCore) { global $config; @@ -34,6 +34,7 @@ file_put_contents($env['FLOWVIEW_FIXTURE_NATIVE_HELPER'], 'fixtureRoot . '/result.json'; $process = proc_open($command, [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes, $this->fixtureRoot . '/plugins/flowview', $env); @@ -345,3 +346,19 @@ ->and(array_column($result['calls'], 0))->not->toContain('delivery', 'save-log'); } })->with(['notification-read-failure', 'notification-missing', 'notification-success']); + + +it('completes CC-only and BCC-only lists through the actual schedule child', function ($field, $nativeCore) { + $recipients = [$field => 'recipient@example.invalid']; + $command = [PHP_BINARY, $this->fixtureRoot . '/plugins/flowview/run_schedule.php', '--report-id=7', '--queue-id=51']; + [$exit] = ($this->runFixture)($command, 'notification-success', $nativeCore, $recipients); + $result = json_decode(file_get_contents($this->fixtureRoot . '/result.json'), true); + $writes = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'write')); + expect($exit)->toBe(0)->and($result['native_helper_loaded'])->toBe($nativeCore) + ->and($writes)->toHaveCount(2)->and($writes[0][1])->toContain('UPDATE plugin_flowview_schedules') + ->and($writes[1][2])->toBe(['complete', 51, 'flowview', 'running']); + $delivery = array_values(array_filter($result['calls'], fn ($c) => $c[0] === 'delivery')); + expect($delivery)->toHaveCount(1)->and($delivery[0][1])->toBe([ + '', $recipients['cc_emails'] ?? '', $recipients['bcc_emails'] ?? '' + ]); +})->with(['cc_emails', 'bcc_emails'])->with([false, true]); diff --git a/tests/Unit/ReportDeliveryTest.php b/tests/Unit/ReportDeliveryTest.php index 07a802c..18fe914 100644 --- a/tests/Unit/ReportDeliveryTest.php +++ b/tests/Unit/ReportDeliveryTest.php @@ -68,3 +68,29 @@ [false, false], [[], false], [['notification' => '', 'name' => 'Fixture', 'request_type' => 'scheduled', 'requested_by' => 'fixture', 'requested_id' => 1], true] ]); + + +it('preserves every supported To CC and BCC recipient combination', function ($type, $mask, $mailError) { + $to = ($mask & 1) ? 'to@example.invalid' : ''; + $cc = ($mask & 2) ? 'cc@example.invalid' : ''; + $bcc = ($mask & 4) ? 'bcc@example.invalid' : ''; + $notification = $type === 'email' + ? ['email' => ['to_email' => $to, 'cc_email' => $cc, 'bcc_email' => $bcc]] + : ['notification_list' => ['id' => 1]]; + plugin_test_queue_db_result('db_fetch_row_prepared', [ + 'name' => 'Fixture', 'notification' => json_encode($notification), 'request_type' => 'scheduled', + 'requested_by' => 'fixture', 'requested_id' => 1 + ]); + plugin_test_queue_db_result('db_fetch_row_prepared', ['emails' => $to, 'cc_emails' => $cc, 'bcc_emails' => $bcc, 'format_file' => 'default']); + plugin_test_queue_db_result('mailer', $mailError); + plugin_test_queue_db_result('sql_save', 1); + $data = []; $raw = ''; $html = ''; $text = ''; + expect(flowview_reports_log_and_notify(51, microtime(true), 'html', 'flowview', 7, 'Fixture', $data, $raw, $html, $text))->toBe($mask && $mailError === '' ? true : null); + $mail = array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'mailer')); + if ($mask) { + expect($mail)->toHaveCount(1)->and(array_slice($mail[0]['params'], 1, 3))->toBe([$to, $cc, $bcc]); + } else { + expect($mail)->toBe([]); + } + expect($GLOBALS['__test_saved_rows'])->toHaveCount($mask && $mailError === '' ? 1 : 0); +})->with(['email', 'notification_list'])->with(range(0, 7))->with(['', 'SMTP failed']); diff --git a/tests/fixtures/filter-lifecycle.php b/tests/fixtures/filter-lifecycle.php index de86429..5a8351d 100644 --- a/tests/fixtures/filter-lifecycle.php +++ b/tests/fixtures/filter-lifecycle.php @@ -86,11 +86,13 @@ function db_fetch_row_prepared($sql, $params = [], ...$args) { if (count($reads) > 1 && $GLOBALS['mode'] === 'notification-read-failure') { return false; } if (count($reads) > 1 && $GLOBALS['mode'] === 'notification-missing') { return []; } if (in_array($GLOBALS['mode'], ['notification-after-delivery', 'notification-mail-error', 'notification-log-error', 'notification-success', 'notification-list-error'], true)) { - return ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'name' => 'Fixture', 'notification' => $GLOBALS['mode'] === 'notification-list-error' ? '{"notification_list":{"id":1}}' : '{"email":{"to_email":"fixture@example.invalid"}}', 'request_type' => 'scheduled', 'requested_by' => 'fixture', 'requested_id' => 1]; + return ['id' => 51, 'source' => 'flowview', 'source_id' => 7, 'name' => 'Fixture', 'notification' => ($GLOBALS['mode'] === 'notification-list-error' || getenv('FLOWVIEW_FIXTURE_RECIPIENTS')) ? '{"notification_list":{"id":1}}' : '{"email":{"to_email":"fixture@example.invalid"}}', 'request_type' => 'scheduled', 'requested_by' => 'fixture', 'requested_id' => 1]; } return ['id' => 51, 'source' => 'flowview', 'source_id' => 7]; } - if (str_contains($sql, 'plugin_notification_lists')) { return ['emails' => 'list@example.invalid', 'format_file' => 'default']; } + if (str_contains($sql, 'plugin_notification_lists')) { + return getenv('FLOWVIEW_FIXTURE_RECIPIENTS') ? json_decode(getenv('FLOWVIEW_FIXTURE_RECIPIENTS'), true) : ['emails' => 'list@example.invalid', 'format_file' => 'default']; + } if (str_contains($sql, 'plugin_flowview_schedules')) { return ['id' => 7, 'query_id' => 3, 'title' => 'Fixture', 'format_file' => 'default']; } @@ -137,7 +139,7 @@ function db_connect_real(...$args) { function reports_load_format_file($format, &$output, &$tag, &$theme) { $output = ''; $tag = true; return true; } function mailer(...$args) { - $GLOBALS['calls'][] = ['delivery']; + $GLOBALS['calls'][] = ['delivery', array_slice($args, 1, 3)]; if ($GLOBALS['mode'] === 'notification-after-delivery') { throw new RuntimeException('Ambiguous delivery failure'); } return in_array($GLOBALS['mode'], ['notification-mail-error', 'notification-list-error'], true) ? 'SMTP delivery failed' : ''; }