Skip to content

Commit 64d8da2

Browse files
sgiehlsgiehl
authored
Introduce new type-safe methods for receiving config section values (#24404)
* Introduce type safe config getters * adjust method usages of section configs * deduplicate float parsing * rename getBooleanConfigValue to getBoolConfigValue * add condition phpstan return types * apply review feedback
1 parent 7479076 commit 64d8da2

24 files changed

Lines changed: 468 additions & 85 deletions

File tree

‎core/Common.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
*/
2929
class Common
3030
{
31+
private const FLOAT_REGEX = "/^[-+]?((([0-9]+(_[0-9]+)*)|(([0-9]+(_[0-9]+)*)?\.([0-9]+(_[0-9]+)*))|(([0-9]+(_[0-9]+)*)\.([0-9]+(_[0-9]+)*)?))([eE][+-]?([0-9]+(_[0-9]+)*))?)$/";
32+
3133
// constants used to map the referrer type to an integer in the log_visit table
3234
public const REFERRER_TYPE_DIRECT_ENTRY = 1;
3335
public const REFERRER_TYPE_SEARCH_ENGINE = 2;
@@ -1093,6 +1095,26 @@ public static function forceDotAsSeparatorForDecimalPoint($value)
10931095
return str_replace(',', '.', $value);
10941096
}
10951097

1098+
/**
1099+
* Parses the given value as float and returns null if it cannot be represented as a PHP float.
1100+
*
1101+
* Supports the same string notations as PHP floats, including underscore notation.
1102+
*
1103+
* @param mixed $value
1104+
*/
1105+
public static function parseFloat($value): ?float
1106+
{
1107+
if (is_float($value) || is_int($value)) {
1108+
return (float)$value;
1109+
}
1110+
1111+
if (is_string($value) && preg_match(self::FLOAT_REGEX, $value)) {
1112+
return (float)str_replace('_', '', $value);
1113+
}
1114+
1115+
return null;
1116+
}
1117+
10961118
/**
10971119
* Sets outgoing header.
10981120
*

‎core/Config/SectionConfig.php‎

Lines changed: 223 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
namespace Piwik\Config;
1111

1212
use Piwik\Config;
13+
use Piwik\Common;
14+
use Piwik\Container\StaticContainer;
15+
use Piwik\Log\LoggerInterface;
1316

1417
abstract class SectionConfig
1518
{
@@ -31,39 +34,248 @@ public static function setConfigValue(string $name, $value): void
3134
/**
3235
* Get a setting value
3336
*
37+
* Prefer one of the type-safe getters if a specific type is expected:
38+
* @see getIntegerConfigValue()
39+
* @see getFloatConfigValue()
40+
* @see getBoolConfigValue()
41+
* @see getStringConfigValue()
42+
* @see getArrayConfigValue()
43+
*
3444
* @param string $name Setting name
3545
* @param int|null $idSite Optional site Id
3646
*
3747
* @return mixed|null
3848
*/
3949
public static function getConfigValue(string $name, ?int $idSite = null)
4050
{
41-
$config = self::getConfig();
42-
if (!empty($idSite)) {
43-
$siteSpecificConfig = self::getSiteSpecificConfig($idSite);
44-
$config = array_merge($config, $siteSpecificConfig);
45-
}
51+
return static::getRawConfigValue($name, $idSite);
52+
}
53+
54+
/**
55+
* @phpstan-return ($default is null ? int|null : int)
56+
*/
57+
public static function getIntegerConfigValue(string $name, ?int $default = null, ?int $idSite = null): ?int
58+
{
59+
return self::castIntConfigValue($name, static::getRawConfigValue($name, $idSite), $default);
60+
}
61+
62+
/**
63+
* @phpstan-return ($default is null ? float|null : float)
64+
*/
65+
public static function getFloatConfigValue(string $name, ?float $default = null, ?int $idSite = null): ?float
66+
{
67+
return self::castFloatConfigValue($name, static::getRawConfigValue($name, $idSite), $default);
68+
}
69+
70+
/**
71+
* @phpstan-return ($default is null ? bool|null : bool)
72+
*/
73+
public static function getBoolConfigValue(string $name, ?bool $default = null, ?int $idSite = null): ?bool
74+
{
75+
return self::castBoolConfigValue($name, static::getRawConfigValue($name, $idSite), $default);
76+
}
77+
78+
/**
79+
* @phpstan-return ($default is null ? string|null : string)
80+
*/
81+
public static function getStringConfigValue(string $name, ?string $default = null, ?int $idSite = null): ?string
82+
{
83+
return self::castStringConfigValue($name, static::getRawConfigValue($name, $idSite), $default);
84+
}
85+
86+
/**
87+
* @return array<mixed>|null
88+
* @phpstan-return ($default is null ? array<mixed>|null : array<mixed>)
89+
*/
90+
public static function getArrayConfigValue(string $name, ?array $default = null, ?int $idSite = null): ?array
91+
{
92+
return self::castArrayConfigValue($name, static::getRawConfigValue($name, $idSite), $default);
93+
}
94+
95+
/**
96+
* @return mixed|null
97+
*/
98+
protected static function getRawConfigValue(string $name, ?int $idSite = null)
99+
{
100+
$config = self::getMergedConfig($idSite);
46101
return $config[$name] ?? null;
47102
}
48103

49104
/**
50105
* Get the section config as an array
51106
*
52-
* @return array|string
107+
* @return array<string, mixed>
53108
*/
54-
private static function getConfig()
109+
private static function getConfig(): array
55110
{
56-
return Config::getInstance()->{static::getSectionName()};
111+
$config = Config::getInstance()->{static::getSectionName()};
112+
return is_array($config) ? $config : [];
57113
}
58114

59115
/**
60116
* Get the site specific config (if any) as an array
61117
*
62-
* @return array|string
118+
* @return array<string, mixed>
63119
*/
64-
private static function getSiteSpecificConfig(int $idSite)
120+
private static function getSiteSpecificConfig(int $idSite): array
65121
{
66122
$key = static::getSectionName() . '_' . $idSite;
67-
return Config::getInstance()->$key;
123+
$config = Config::getInstance()->$key;
124+
return is_array($config) ? $config : [];
125+
}
126+
127+
/**
128+
* @return array<string, mixed>
129+
*/
130+
private static function getMergedConfig(?int $idSite = null): array
131+
{
132+
$config = self::getConfig();
133+
if (!empty($idSite)) {
134+
$config = array_merge($config, self::getSiteSpecificConfig($idSite));
135+
}
136+
return $config;
137+
}
138+
139+
/**
140+
* @param mixed $value
141+
*/
142+
private static function castIntConfigValue(string $name, $value, ?int $default): ?int
143+
{
144+
if ($value === null) {
145+
return $default;
146+
}
147+
148+
if ((is_string($value) || is_numeric($value)) && (string) $value === (string) (int) $value) {
149+
return (int) $value;
150+
}
151+
152+
self::logTypeCastWarning($name, 'int', $value);
153+
return $default;
154+
}
155+
156+
/**
157+
* @param mixed $value
158+
*/
159+
private static function castFloatConfigValue(string $name, $value, ?float $default): ?float
160+
{
161+
if ($value === null) {
162+
return $default;
163+
}
164+
165+
$parsedFloat = Common::parseFloat($value);
166+
if ($parsedFloat !== null) {
167+
return $parsedFloat;
168+
}
169+
170+
self::logTypeCastWarning($name, 'float', $value);
171+
return $default;
172+
}
173+
174+
/**
175+
* @param mixed $value
176+
*/
177+
private static function castBoolConfigValue(string $name, $value, ?bool $default): ?bool
178+
{
179+
if ($value === null) {
180+
return $default;
181+
}
182+
183+
if ($value === false || $value === true) {
184+
return $value;
185+
}
186+
187+
if ((is_string($value) && strtolower($value) === 'false') || $value === '0' || $value === 0) {
188+
return false;
189+
}
190+
191+
if ((is_string($value) && strtolower($value) === 'true') || $value === '1' || $value === 1) {
192+
return true;
193+
}
194+
195+
self::logTypeCastWarning($name, 'bool', $value);
196+
return $default;
197+
}
198+
199+
/**
200+
* @param mixed $value
201+
*/
202+
private static function castStringConfigValue(string $name, $value, ?string $default): ?string
203+
{
204+
if ($value === null) {
205+
return $default;
206+
}
207+
208+
if (is_string($value) || is_numeric($value)) {
209+
return Common::sanitizeNullBytes((string) $value);
210+
}
211+
212+
self::logTypeCastWarning($name, 'string', $value);
213+
return $default;
214+
}
215+
216+
/**
217+
* @param mixed $value
218+
* @return array<mixed>|null
219+
*/
220+
private static function castArrayConfigValue(string $name, $value, ?array $default): ?array
221+
{
222+
if ($value === null) {
223+
return $default;
224+
}
225+
226+
if (is_array($value)) {
227+
/** @var array<mixed> $sanitizedValue */
228+
$sanitizedValue = self::filterNullBytes($value);
229+
return $sanitizedValue;
230+
}
231+
232+
self::logTypeCastWarning($name, 'array', $value);
233+
return $default;
234+
}
235+
236+
/**
237+
* @param mixed $value
238+
*/
239+
private static function logTypeCastWarning(string $name, string $type, $value): void
240+
{
241+
StaticContainer::get(LoggerInterface::class)->warning(
242+
'Failed to cast config value {section}.{name} to {type}; actual type was {actualType}.',
243+
[
244+
'section' => static::getSectionName(),
245+
'name' => $name,
246+
'type' => $type,
247+
'actualType' => self::getValueType($value),
248+
]
249+
);
250+
}
251+
252+
/**
253+
* @param mixed $value
254+
*/
255+
private static function getValueType($value): string
256+
{
257+
if (is_object($value)) {
258+
return get_class($value);
259+
}
260+
261+
return gettype($value);
262+
}
263+
264+
/**
265+
* @param mixed $value
266+
* @return mixed
267+
*/
268+
private static function filterNullBytes($value)
269+
{
270+
if (is_array($value)) {
271+
$result = [];
272+
foreach ($value as $key => $arrayValue) {
273+
$result[$key] = self::filterNullBytes($arrayValue);
274+
}
275+
276+
return $result;
277+
}
278+
279+
return is_string($value) ? Common::sanitizeNullBytes($value) : $value;
68280
}
69281
}

‎core/DataTable/Filter/PivotByDimension.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ public static function isPivotingReportBySubtableSupported(Report $report)
581581
*/
582582
public static function isSegmentFetchingEnabledInConfig()
583583
{
584-
return (bool)GeneralConfig::getConfigValue('pivot_by_filter_enable_fetch_by_segment');
584+
return GeneralConfig::getBoolConfigValue('pivot_by_filter_enable_fetch_by_segment', false);
585585
}
586586

587587
/**
@@ -592,7 +592,7 @@ public static function isSegmentFetchingEnabledInConfig()
592592
*/
593593
public static function getDefaultColumnLimit()
594594
{
595-
return (int)GeneralConfig::getConfigValue('pivot_by_filter_default_column_limit');
595+
return GeneralConfig::getIntegerConfigValue('pivot_by_filter_default_column_limit', 0);
596596
}
597597

598598
/**

‎core/Request.php‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -156,17 +156,9 @@ public function getIntegerParameter(string $name, ?int $default = null): int
156156
public function getFloatParameter(string $name, ?float $default = null): float
157157
{
158158
$parameter = $this->getParameter($name, $default);
159-
160-
if (is_float($parameter) || is_int($parameter)) {
161-
return (float)$parameter;
162-
}
163-
164-
// Regex for all supported float notations in PHP (see https://www.php.net/manual/en/language.types.float.php)
165-
$floatRegex = "/^[-+]?((([0-9]+(_[0-9]+)*)|(([0-9]+(_[0-9]+)*)?\.([0-9]+(_[0-9]+)*))|(([0-9]+(_[0-9]+)*)\.([0-9]+(_[0-9]+)*)?))([eE][+-]?([0-9]+(_[0-9]+)*))?)$/";
166-
167-
if (is_string($parameter) && preg_match($floatRegex, $parameter)) {
168-
// underscores would break numbers if not removed before
169-
return (float) str_replace('_', '', $parameter);
159+
$parsedFloat = Common::parseFloat($parameter);
160+
if ($parsedFloat !== null) {
161+
return $parsedFloat;
170162
}
171163

172164
if (null !== $default) {

‎core/SiteContentDetector.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ private function requestSiteResponse(string $url, int $timeOut): array
347347
}
348348

349349
// If internet features are disabled, we don't try to fetch any site content
350-
if (0 === (int) GeneralConfig::getConfigValue('enable_internet_features')) {
350+
if (0 === GeneralConfig::getIntegerConfigValue('enable_internet_features', 0)) {
351351
return [];
352352
}
353353

‎core/Tracker.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -377,12 +377,12 @@ private function handleFatalErrors()
377377
private static function isDebugEnabled()
378378
{
379379
try {
380-
$debug = (bool) TrackerConfig::getConfigValue('debug');
380+
$debug = TrackerConfig::getBoolConfigValue('debug', false);
381381
if ($debug) {
382382
return true;
383383
}
384384

385-
$debugOnDemand = (bool) TrackerConfig::getConfigValue('debug_on_demand');
385+
$debugOnDemand = TrackerConfig::getBoolConfigValue('debug_on_demand', false);
386386
if ($debugOnDemand) {
387387
return (bool) Common::getRequestVar('debug', false);
388388
}

‎core/Tracker/Request.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,9 @@ public function __construct(
100100
// check for 4byte utf8 characters in all tracking params and replace them with � if not support by database
101101
$this->params = $this->replaceUnsupportedUtf8Chars($this->params);
102102

103-
$this->customTimestampDoesNotRequireTokenauthWhenNewerThan = (int) TrackerConfig::getConfigValue(
103+
$this->customTimestampDoesNotRequireTokenauthWhenNewerThan = TrackerConfig::getIntegerConfigValue(
104104
'tracking_requests_require_authentication_when_custom_timestamp_newer_than',
105+
0,
105106
$this->getIdSiteIfExists()
106107
);
107108
}

0 commit comments

Comments
 (0)
⚡