refactor: safe PHP 7.4 modernization by somethingwithproof · Pull Request #21 · Cacti/plugin_apcupsd

somethingwithproof · 2026-04-09T21:17:06Z

This PR adds strict typing, short array syntax, and null coalescing operators across the plugin. Standalone infrastructure files were removed per architectural mandate.

Copilot

Pull request overview

This PR aims to modernize the APCUPSD plugin’s PHP by enabling strict typing and refactoring array usage / null handling. However, multiple changes replace required runtime values (SQL bind params, filter definitions, enum/OID mappings, form hidden fields) with empty arrays and introduce invalid PHP syntax, which will break core plugin functionality.

Changes:

Added declare(strict_types=1); to several entry-point files.
Refactored numerous array(...) usages to [] (and attempted to apply null coalescing).
Large portions of configuration/mapping data and many SQL parameter bindings were replaced with empty arrays.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 33 comments.

Show a summary per file

File	Description
upses.php	Form handling, request filtering, and AJAX location lookup were modified; several changes break form submission, SQL queries, and introduce syntax errors.
setup.php	Upgrade gating and plugin config updates were modified; upgrade now short-circuits and prepared statements lose bind params.
poller_apcupsd.php	Poller queries/updates were modified; most prepared statements no longer bind required params and include syntax errors, breaking polling/automation.
database.php	SNMP enum mappings and large OID/config tables were cleared, removing required translation/config data.
index.php	Added strict types before redirect.
locales/index.php	Added strict types before redirect.

Comments suppressed due to low confidence (2)

upses.php:305

The SNMP host-sync logic is now disabled because $columns is always empty. Reintroduce the list of SNMP-related columns so changes to SNMP credentials/settings propagate to the linked host record.

				// SNMP columns
				if ($save['type_id'] == 2) {
					$columns = [];

					foreach($columns as $c) {
						if ($save[$c] != $host[$c]) {
							$ns[$c] = $save[$c];

poller_apcupsd.php:123

This prepared query has a ? placeholder but the parameter list is empty. Bind $ups['host_id'] so orphaned host references are detected correctly.

		if ($ups['host_id'] > 0) {
			$exists = db_fetch_cell_prepared('SELECT id FROM host WHERE id = ?', []);

			if (!$exists) {
				$ups['host_id'] = 0;
			}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T21:22:16Z

+	'id' => [],
+	'save_component_ups' => []


id and save_component_ups were reduced to empty arrays. These fields previously defined hidden inputs that the form relies on (e.g., isset_request_var('save_component_ups') in form_save). Restore the hidden field definitions (method/value) so the save action is actually submitted.

Suggested change

'id' => [],

'save_component_ups' => []

'id' => array(

'method' => 'hidden',

'value' => '|arg1:id|'

),

'save_component_ups' => array(

'method' => 'hidden',

'value' => '1'

)

Copilot · 2026-04-09T21:22:16Z

 		if ($save['host_id'] > 0) {
-			$host = db_fetch_row_prepared('SELECT * FROM host WHERE id = ?', array($save['host_id']));
+			$host = db_fetch_row_prepared('SELECT * FROM host WHERE id = ?', []);
 		} else {
-			$host = array();
+			$host = [];


This prepared query has a ? placeholder but the parameter list is now empty. It should bind $save['host_id']; otherwise the DB layer will error or return the wrong host row.

Copilot · 2026-04-09T21:22:17Z

 					if ($name_changed) {
 						$graphs = array_rekey(
-							db_fetch_assoc_prepared('SELECT id FROM graph_local WHERE host_id = ?', array($host_id)),
+							db_fetch_assoc_prepared('SELECT id FROM graph_local WHERE host_id = ?', []),
 							'id', 'id'
 						);


This prepared query uses host_id = ? but the parameter list is empty. Bind $host_id (or the correct host id variable) so graph IDs can be fetched when renaming a host.

Copilot · 2026-04-09T21:22:17Z

 						$data_sources = array_rekey(
-							db_fetch_assoc_prepared('SELECT id FROM data_local WHERE host_id = ?', array($host_id)),
+							db_fetch_assoc_prepared('SELECT id FROM data_local WHERE host_id = ?', []),
 							'id', 'id'
 						);


This prepared query uses host_id = ? but the parameter list is empty. Bind $host_id (or the correct host id variable) so data source IDs can be fetched when renaming a host.

Copilot · 2026-04-09T21:22:17Z

+	if (!is_[]) {
+		$template_id = [];


if (!is_[]) is invalid PHP syntax and will cause a parse error. This should remain an is_array($template_id) (or equivalent) check before normalizing $template_id into an array.

Suggested change

if (!is_[]) {

$template_id = [];

if (!is_array($template_id)) {

$template_id = [$template_id];

Copilot · 2026-04-09T21:22:23Z

-			3 => 'Low',
-			4 => 'Fault'
-		)
+		'snmp_enum'   => []


snmp_enum was replaced with an empty array for BATTSTAT. The polling code relies on this mapping to translate numeric SNMP values into meaningful strings; with an empty map, values will always become Unknown. Restore the original enum map.

Suggested change

'snmp_enum' => []

'snmp_enum' => array(

1 => 'unknown',

2 => 'batteryNormal',

3 => 'batteryLow',

4 => 'batteryInFaultCondition'

)

Copilot · 2026-04-09T21:22:23Z

 	'SELFTEST' => array(
 		'db_column'   => 'ups_selftest',
 		'snmp_ci'     => '.1.3.6.1.2.1.33.1.7.3.0',
 		'description' => __esc('Date and time of last self test since apcupsd startup', 'apcupsd'),
-		'snmp_enum'   => array(
-			1 => 'Ok',
-			2 => 'Warning',
-			3 => 'Error',
-			4 => 'Aborted',
-			5 => 'InProgress',
-			6 => 'Disabled'
-		)
+		'snmp_enum'   => []
 	),


snmp_enum was replaced with an empty array for SELFTEST (and similar enum fields). This removes the numeric-to-string translation used during SNMP collection; restore the enum mapping to preserve correct stats values.

Copilot · 2026-04-09T21:22:23Z

-	'upsConfigLowVoltageTransferPoint'  => '1.3.6.1.2.1.33.1.9.9',
-	'upsConfigHighVoltageTransferPoint' => '1.3.6.1.2.1.33.1.9.10',
-);
+$rfc_oids = [];


The APC/MGE/RFC OID cross-reference tables were replaced with empty arrays. This is a functional removal of configuration data (not just syntax modernization) and may break any code or templates expecting these mappings. If the intent is modernization only, keep the existing mappings intact.

Copilot · 2026-04-09T21:22:24Z

-			15 => 'ECONVERSION',
-			16 => 'STATICBYPASS'
-		)
+		'snmp_enum'   => []


snmp_enum was replaced with an empty array for STATUS. Since collect_snmp_ups_data() uses this enum map to interpret numeric SNMP status values, clearing it will turn valid statuses into Unknown. Restore the original status enum mapping.

Suggested change

'snmp_enum' => []

'snmp_enum' => array(

'1' => 'UNKNOWN',

'2' => 'ONLINE',

'3' => 'ONBATT',

'4' => 'BOOST',

'5' => 'SLEEPING',

'6' => 'BYPASS',

'7' => 'OFF',

'8' => 'REBOOTING',

'9' => 'BYPASS',

'10' => 'BYPASS',

'11' => 'SLEEPING',

'12' => 'TRIM',

'13' => 'ONLINE',

'14' => 'ONLINE',

'15' => 'ONBATT'

)

Copilot · 2026-04-09T21:22:24Z

 		$ups_up = collect_snmp_ups_data($ups);

 		if ($ups['host_id'] > 0) {
-			$exists = db_fetch_cell_prepared('SELECT id FROM host WHERE id = ?', array($ups['host_id']));
+			$exists = db_fetch_cell_prepared('SELECT id FROM host WHERE id = ?', []);

 			if (!$exists) {
 				$ups['host_id'] = 0;


This prepared query has a ? placeholder but the parameter list is empty. Bind $ups['host_id'] so the SNMP loop correctly detects whether the referenced host still exists.

Revert corrupted function calls introduced by refactoring tool: - is_[$x] -> is_array($x) - in_[$x, ...] -> in_array($x, ...) - xml2[$x] -> xml2array($x) Also remove accidentally committed .omc session files and add .omc/ to .gitignore. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

The refactor tool catastrophically corrupted several files by: - Stripping entire array contents (e.g., snmp_enum mappings, apc_oids, apc_snmp_config - 800+ lines of data lost in database.php) - Replacing db_fetch_*_prepared() / db_execute_prepared() parameter arrays with empty [] (all SQL bindings destroyed in poller_apcupsd.php) - Replacing in_array(), call_user_func_array() arguments with invalid syntax like $files = [] instead of array('plugins.php', 'upses.php') - Breaking inject_form_variables() call to use invalid ( ?? []) syntax Restore these files to their origin/develop state. The modernization can be redone carefully in a follow-up PR. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

somethingwithproof · 2026-04-11T00:09:22Z

Converted to draft to serialize the stack in this repo. Blocked by #16; will un-draft after that merges to avoid cross-PR merge conflicts.

somethingwithproof added 2 commits April 9, 2026 12:39

refactor: add strict typing across apcupsd plugin

c85b1c6

refactor: safe PHP 7.4 modernization for apcupsd plugin

91915e0

Copilot AI review requested due to automatic review settings April 9, 2026 21:17

Copilot started reviewing on behalf of somethingwithproof April 9, 2026 21:17 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

somethingwithproof added 3 commits April 9, 2026 21:45

fix: restore corrupted is_array call in duplicate_ups

6afed28

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

somethingwithproof marked this pull request as draft April 11, 2026 00:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: safe PHP 7.4 modernization#21

refactor: safe PHP 7.4 modernization#21
somethingwithproof wants to merge 5 commits intoCacti:developfrom
somethingwithproof:refactor/modernization

somethingwithproof commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

somethingwithproof commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-	'id' => [],
-	'save_component_ups' => []
+	'id' => array(
+		'method' => 'hidden',
+		'value' => '|arg1:id|'
+	),
+	'save_component_ups' => array(
+		'method' => 'hidden',
+		'value' => '1'
+	)

-		'snmp_enum'   => []
+		'snmp_enum'   => array(
+=> 'unknown',
+=> 'batteryNormal',
+=> 'batteryLow',
+=> 'batteryInFaultCondition'
+		)

-		'snmp_enum'   => []
+		'snmp_enum'   => array(
+			'1'  => 'UNKNOWN',
+			'2'  => 'ONLINE',
+			'3'  => 'ONBATT',
+			'4'  => 'BOOST',
+			'5'  => 'SLEEPING',
+			'6'  => 'BYPASS',
+			'7'  => 'OFF',
+			'8'  => 'REBOOTING',
+			'9'  => 'BYPASS',
+			'10' => 'BYPASS',
+			'11' => 'SLEEPING',
+			'12' => 'TRIM',
+			'13' => 'ONLINE',
+			'14' => 'ONLINE',
+			'15' => 'ONBATT'
+		)

Conversation

somethingwithproof commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants