fix(thold): keep zero readings and stop falling back to the wrong data source

.github/workflows/plugin-ci-workflow.yml

@@ -35,21 +35,12 @@ jobs:
   integration-test:
     runs-on: ${{ matrix.os }}
 
-    # A failure against the pinned release is a real failure. The develop entry
-    # is advisory: it is how a core regression becomes visible here, but it must
-    # not turn the plugin's own pull requests red.
-    continue-on-error: ${{ matrix.cacti != 'release/1.2.31' }}
-
     strategy:
       fail-fast: false
       matrix:
         php: ['8.1', '8.2', '8.3', '8.4']
         os: [ubuntu-latest]
         cacti: ['release/1.2.31']
-        include:
-          - php: '8.4'
-            os: ubuntu-latest
-            cacti: 'develop'
 
     services:
       mariadb:
@@ -95,7 +86,24 @@ jobs:
         echo "PHP_BINARY=$(command -v php)" >> "$GITHUB_ENV"
 
     - name: Run apt-get update
-      run: sudo apt-get update
+      run: |
+        for attempt in 1 2 3; do
+          if sudo timeout 3m apt-get \
+            -o Dpkg::Lock::Timeout=60 \
+            -o Acquire::Retries=3 \
+            -o Acquire::http::Timeout=30 \
+            -o Acquire::https::Timeout=30 \
+            update; then
+            exit 0
+          fi
+
+          if [ "$attempt" -lt 3 ]; then
+            sleep 10
+          fi
+        done
+
+        echo 'apt-get update failed after three bounded attempts.' >&2
+        exit 1
 
     - name: Install System Dependencies
       run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping

tests/Unit/GetCurrentValueTest.php

@@ -0,0 +1,148 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ |                                                                         |
+ | This program is free software; you can redistribute it and/or           |
+ | modify it under the terms of the GNU General Public License             |
+ | as published by the Free Software Foundation; either version 2          |
+ | of the License, or (at your option) any later version.                  |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+ | http://www.cacti.net/                                                   |
+ +-------------------------------------------------------------------------+
+*/
+
+/**
+ * get_current_value() reads one data source's latest value out of the RRD.
+ *
+ * Its contract on failure is to return 0. Returning some other data source's
+ * value instead is worse than returning nothing, because the caller compares
+ * whatever it gets against the threshold bounds.
+ */
+final class GetCurrentValueTest extends TestCase {
+	/**
+	 * @return void
+	 */
+	public static function setUpBeforeClass(): void {
+		self::loadPluginSource('thold_functions.php');
+	}
+
+	/**
+	 * @return void
+	 */
+	protected function setUp(): void {
+		parent::setUp();
+
+		CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]);
+
+		// thold_rrd_last() returns whatever `rrdtool last` printed.
+		CactiStubs::willReturn('rrdtool_execute', '1700000000');
+	}
+
+	/**
+	 * @param array<string, mixed> $fetch
+	 *
+	 * @return void
+	 */
+	private function rrdReturns(array $fetch) {
+		CactiStubs::willReturn('rrdtool_function_fetch', $fetch);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testTheRequestedDataSourceValueIsReturned(): void {
+		$this->rrdReturns([
+			'data_source_names' => ['traffic_in', 'traffic_out'],
+			'values'            => [['1700000000' => 10.0], ['1700000000' => 20.0]],
+		]);
+
+		$this->assertSame(20.0, get_current_value(4, 'traffic_out'));
+	}
+
+	/**
+	 * array_search() returns false, not null, so a guard written against null
+	 * let the miss through and PHP then read index 0 — the first data source.
+	 *
+	 * @return void
+	 */
+	public function testUnknownDataSourceReturnsZeroRatherThanTheFirstOne(): void {
+		$this->rrdReturns([
+			'data_source_names' => ['traffic_in', 'traffic_out'],
+			'values'            => [['1700000000' => 10.0], ['1700000000' => 20.0]],
+		]);
+
+		$this->assertSame(0, get_current_value(4, 'upper_limit'));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testFirstDataSourceIsStillReachableByName(): void {
+		$this->rrdReturns([
+			'data_source_names' => ['traffic_in', 'traffic_out'],
+			'values'            => [['1700000000' => 10.0], ['1700000000' => 20.0]],
+		]);
+
+		$this->assertSame(10.0, get_current_value(4, 'traffic_in'));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testMissingDataSourceNamesReturnsZero(): void {
+		$this->rrdReturns([]);
+
+		$this->assertSame(0, get_current_value(4, 'traffic_in'));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testMissingValuesReturnsZero(): void {
+		$this->rrdReturns(['data_source_names' => ['traffic_in']]);
+
+		$this->assertSame(0, get_current_value(4, 'traffic_in'));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testEmptyValueSeriesReturnsZero(): void {
+		$this->rrdReturns([
+			'data_source_names' => ['traffic_in'],
+			'values'            => [[]],
+		]);
+
+		$this->assertSame(0, get_current_value(4, 'traffic_in'));
+	}
+
+	/**
+	 * A missing or unreadable RRD makes `rrdtool last` print nothing, which
+	 * used to reach the timestamp arithmetic as an empty string and fatal.
+	 *
+	 * @return void
+	 */
+	public function testUnreadableRrdReturnsZeroRatherThanThrowing(): void {
+		CactiStubs::reset();
+		CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]);
+		CactiStubs::willReturn('rrdtool_execute', '');
+		$this->rrdReturns([]);
+
+		$this->assertSame(0, get_current_value(4, 'traffic_in'));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testValueIsRoundedToFourDecimals(): void {
+		$this->rrdReturns([
+			'data_source_names' => ['traffic_in'],
+			'values'            => [['1700000000' => 1.23456789]],
+		]);
+
+		$this->assertSame(1.2346, get_current_value(4, 'traffic_in'));
+	}
+}

tests/Unit/TholdStrReplaceTest.php

@@ -0,0 +1,94 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ |                                                                         |
+ | This program is free software; you can redistribute it and/or           |
+ | modify it under the terms of the GNU General Public License             |
+ | as published by the Free Software Foundation; either version 2          |
+ | of the License, or (at your option) any later version.                  |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+ | http://www.cacti.net/                                                   |
+ +-------------------------------------------------------------------------+
+*/
+
+/**
+ * thold_str_replace() performs every tag substitution in notification bodies,
+ * subjects and trigger commands.
+ *
+ * It exists to turn an absent value into an empty string rather than printing
+ * the word "null". Zero is a real reading and has to survive.
+ */
+final class TholdStrReplaceTest extends TestCase {
+	/**
+	 * @return void
+	 */
+	public static function setUpBeforeClass(): void {
+		self::loadPluginSource('thold_functions.php');
+	}
+
+	/**
+	 * @return array<string, array{0: mixed, 1: string}>
+	 */
+	public static function preservedValueProvider() {
+		return [
+			'integer zero'        => [0, 'v=0'],
+			'string zero'         => ['0', 'v=0'],
+			'float zero'          => [0.0, 'v=0'],
+			'negative'            => [-5, 'v=-5'],
+			'positive integer'    => [5, 'v=5'],
+			'float'               => [2.5, 'v=2.5'],
+			'string zero decimal' => ['0.0', 'v=0.0'],
+		];
+	}
+
+	/**
+	 * @dataProvider preservedValueProvider
+	 *
+	 * @param mixed  $replace
+	 * @param string $expected
+	 *
+	 * @return void
+	 */
+	public function testNumericValuesSurviveSubstitution($replace, $expected): void {
+		$this->assertSame($expected, thold_str_replace('<X>', $replace, 'v=<X>'));
+	}
+
+	/**
+	 * @return array<string, array{0: mixed}>
+	 */
+	public static function absentValueProvider() {
+		return [
+			'null'         => [null],
+			'false'        => [false],
+			'empty string' => [''],
+		];
+	}
+
+	/**
+	 * @dataProvider absentValueProvider
+	 *
+	 * @param mixed $replace
+	 *
+	 * @return void
+	 */
+	public function testAbsentValuesBecomeEmpty($replace): void {
+		$this->assertSame('v=', thold_str_replace('<X>', $replace, 'v=<X>'));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testEveryOccurrenceIsReplaced(): void {
+		$this->assertSame('0 and 0', thold_str_replace('<X>', 0, '<X> and <X>'));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testSubjectWithoutTheTagIsUnchanged(): void {
+		$this->assertSame('no tags here', thold_str_replace('<X>', 5, 'no tags here'));
+	}
+}

tests/bootstrap-unit.php

@@ -157,7 +157,7 @@ function db_fetch_cell_prepared($sql, $params = [], $col_name = '', $log = true,
 }
 
 if (!function_exists('db_qstr')) {
-	function db_qstr($string) {
+	function db_qstr($string, $db_conn = false) {
 		return "'" . str_replace("'", "''", (string) $string) . "'";
 	}
 }

tests/docker/Dockerfile

@@ -0,0 +1,29 @@
+# Test runner for the Thold plugin.
+#
+# Pinned to PHP 8.1 because that is the oldest interpreter the CI matrix
+# covers; what passes here passes on 8.2-8.4.  pcov rather than Xdebug: line
+# coverage is the only debug feature the suite needs and pcov is far cheaper.
+FROM php:8.1-cli-alpine@sha256:7949370448b0b4d9787776dc5968e0fd8d48763292344b5fbf21539441228a98
+
+# git is needed by the changed-line coverage gate, which diffs against the
+# base branch.
+RUN apk add --no-cache git gmp-dev \
+	&& docker-php-ext-install gmp \
+	&& apk add --no-cache --virtual .build-deps $PHPIZE_DEPS \
+	&& pecl install pcov \
+	&& docker-php-ext-enable pcov \
+	&& apk del .build-deps
+
+COPY --from=composer:2@sha256:4d71c3c2109c61d5415544264b59ad4087e4c5b7244481723664138fd36d5040 /usr/bin/composer /usr/bin/composer
+
+# The plugin lives where Cacti would put it, because thold_functions.php
+# resolves its own includes through $config['base_path'] . '/plugins/thold'.
+# No network or database is involved; the Cacti framework functions themselves
+# are stubbed in tests/bootstrap.php.
+WORKDIR /cacti/plugins/thold
+
+ENV COMPOSER_ALLOW_SUPERUSER=1 \
+    COMPOSER_NO_INTERACTION=1 \
+    COMPOSER_CACHE_DIR=/tmp/composer-cache
+
+CMD ["sh", "-c", "composer install --no-progress --no-ansi && composer test"]

tests/docker/docker-compose.yml

@@ -0,0 +1,15 @@
+# Local mirror of the unit-test CI job.  `docker compose -f
+# tests/docker/docker-compose.yml run --rm phpunit` runs exactly what CI runs.
+services:
+  phpunit:
+    build:
+      context: .
+      dockerfile: Dockerfile
+    image: cacti-thold-test:php8.1
+    working_dir: /cacti/plugins/thold
+    volumes:
+      - ../..:/cacti/plugins/thold
+      - composer-cache:/tmp/composer-cache
+
+volumes:
+  composer-cache: