Jeppe Bandopenhands
a71194d46e
## What
Centralises the truthy-string → bool coercion that six different classes
were reimplementing (and which two implementations disagreed about).
The shared helper lives in
`services/nginx/app/traits/boolean_normalization_t.php`:
```php
namespace traits;
trait boolean_normalization_t {
public static function normalizeBoolean(mixed $value): bool {
if (is_bool($value)) return $value;
return in_array(strtolower(trim((string)$value)), ['1','true','yes','on'], true);
}
}
```
`traits/module_config_variable_t::inputToBool` now delegates to it. The
seven call sites that previously inlined the same expression (or wrapped
it in a private `toBool`/`boolValue`/`isEnabled`) are reduced to a
single `self::normalizeBoolean(...)` call:
| Class | Old helper | New |
| --- | --- | --- |
| `classes/cron_worker.php` | inline in `boolOption` |
`self::normalizeBoolean(...)` (after empty-value short-circuit) |
| `classes/replica_failover_manager.php` | `boolValue` |
`self::normalizeBoolean(...)` |
| `classes/release_manager.php` | `toBool` |
`self::normalizeBoolean(...)` |
| `classes/releasemanager.php` | `isEnabled` |
`self::normalizeBoolean(...)` |
| `classes/superuser_system_status_service.php` | inline in
`parseModuleConfigValue` | `self::normalizeBoolean(...)` |
| `classes/module_usage_service.php` | `toBool` |
`self::normalizeBoolean(...)` |
| `classes/account_deletion_service.php` | inline in `apiEnabled` |
`self::normalizeBoolean(...)` |
| `traits/module_config_variable_t.php` | `inputToBool` (narrow set) |
`self::normalizeBoolean(...)` (full set) |
## Why
The pre-PR repo had two silent bugs:
1. **`inputToBool` accepted only `'true'`/`'1'`** while the six inline
copies accepted the wider `['1','true','yes','on']` set. Config values
such as `"yes"` or `" ON "` would round-trip to `false` through
`inputToBool` but `true` through any of the inline copies. This PR picks
the wider set as the single source of truth; the change is a strict
superset, so no caller flips from truthy to falsy.
2. **Six copies of the same expression** to drift in any of the seven
places (whitespace handling, case sensitivity, empty-string semantics).
One trait replaces them.
## Tests
* `tests/Unit/Traits/BooleanNormalizationTest.php` — Pest, runs the
helper directly through anonymous-class composition (no DB/HTTP).
* `tests/Smoke/boolean_normalization_smoke.php` — standalone PHP smoke
runner for environments without composer installed. Verified locally:
7/7 consumer wiring checks pass, 19/19 normalization cases pass (`true`,
`false`, `1`, `0`, `'true'`, `'TRUE'`, `'1'`, `'yes'`, `'YES'`, `'on'`,
`' ON '`, `'false'`, `'no'`, `'off'`, `''`, `null`, `'0'`, `[]`,
stdClass).
* All eight touched files pass `php -l` syntax check.
## Risk
* Behavioural change is a strict superset for the shared expression
path, so no caller can flip from truthy → falsy. The only consumer that
saw a behaviour change for *negative* inputs is `inputToBool` itself,
which previously rejected `'yes'`/`'on'`. Worth a CI pass on the
unit/integration suites before merge.
## Co-author
Co-authored-by: openhands <openhands@all-hands.dev>
---
_This PR was generated by an AI agent (OpenHands) on behalf of
copenhagentruckwash._
---------
Co-authored-by: openhands <openhands@all-hands.dev>
80 lines
2.7 KiB
PHP
80 lines
2.7 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
use traits\boolean_normalization_t;
|
|
use traits\module_config_variable;
|
|
|
|
it('treats true and integer 1 as truthy, everything else as false', function (): void {
|
|
$subject = new class {
|
|
use boolean_normalization_t;
|
|
};
|
|
|
|
expect($subject::normalizeBoolean(true))->toBeTrue();
|
|
expect($subject::normalizeBoolean(1))->toBeTrue();
|
|
});
|
|
|
|
it('accepts the canonical truthy-string set with whitespace and case folded', function (): void {
|
|
$subject = new class {
|
|
use boolean_normalization_t;
|
|
};
|
|
|
|
foreach (['true', 'TRUE', 'True', '1', 'yes', 'YES', 'Yes', 'on', 'ON', 'On'] as $case) {
|
|
expect($subject::normalizeBoolean($case))->toBeTrue();
|
|
}
|
|
|
|
expect($subject::normalizeBoolean(' true '))->toBeTrue();
|
|
expect($subject::normalizeBoolean(" YES\t"))->toBeTrue();
|
|
});
|
|
|
|
it('treats integer 0 and the canonical falsy strings as false', function (): void {
|
|
$subject = new class {
|
|
use boolean_normalization_t;
|
|
};
|
|
|
|
expect($subject::normalizeBoolean(false))->toBeFalse();
|
|
expect($subject::normalizeBoolean(0))->toBeFalse();
|
|
expect($subject::normalizeBoolean(''))->toBeFalse();
|
|
expect($subject::normalizeBoolean('0'))->toBeFalse();
|
|
expect($subject::normalizeBoolean('false'))->toBeFalse();
|
|
expect($subject::normalizeBoolean('no'))->toBeFalse();
|
|
expect($subject::normalizeBoolean('off'))->toBeFalse();
|
|
});
|
|
|
|
it('treats null, arrays and objects as false', function (): void {
|
|
$subject = new class {
|
|
use boolean_normalization_t;
|
|
};
|
|
|
|
expect($subject::normalizeBoolean(null))->toBeFalse();
|
|
expect($subject::normalizeBoolean([]))->toBeFalse();
|
|
expect($subject::normalizeBoolean(['true']))->toBeFalse();
|
|
expect($subject::normalizeBoolean((object)['value' => 'true']))->toBeFalse();
|
|
});
|
|
|
|
it('keeps module_config_variable::inputToBool returning true for the existing truthy strings', function (): void {
|
|
app_require('traits/module_config_variable_t.php');
|
|
|
|
$subject = new class {
|
|
use module_config_variable;
|
|
};
|
|
|
|
expect($subject::inputToBool('true'))->toBeTrue();
|
|
expect($subject::inputToBool('1'))->toBeTrue();
|
|
expect($subject::inputToBool('false'))->toBeFalse();
|
|
});
|
|
|
|
it('now also accepts the wider truthy-string set through inputToBool (parity with the inline copies)', function (): void {
|
|
app_require('traits/module_config_variable_t.php');
|
|
|
|
$subject = new class {
|
|
use module_config_variable;
|
|
};
|
|
|
|
// These were accepted by the inline in_array(...) copies but rejected
|
|
// by the previous inputToBool implementation. They are now consistent.
|
|
expect($subject::inputToBool('yes'))->toBeTrue();
|
|
expect($subject::inputToBool('on'))->toBeTrue();
|
|
expect($subject::inputToBool(' YES '))->toBeTrue();
|
|
});
|