From 01489846464599a6970ff3fe435ed5ea65c598f2 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 20 Mar 2024 14:58:15 +0100 Subject: [PATCH 1/6] Transfer implementation of JsonResultSet and CsvResultSet to separate traits --- library/Icingadb/Data/CsvResultSet.php | 75 +----------------- library/Icingadb/Data/CsvResultSetUtils.php | 82 ++++++++++++++++++++ library/Icingadb/Data/JsonResultSet.php | 70 +---------------- library/Icingadb/Data/JsonResultSetUtils.php | 77 ++++++++++++++++++ 4 files changed, 161 insertions(+), 143 deletions(-) create mode 100644 library/Icingadb/Data/CsvResultSetUtils.php create mode 100644 library/Icingadb/Data/JsonResultSetUtils.php diff --git a/library/Icingadb/Data/CsvResultSet.php b/library/Icingadb/Data/CsvResultSet.php index 746a7e4a3..1e6d7c546 100644 --- a/library/Icingadb/Data/CsvResultSet.php +++ b/library/Icingadb/Data/CsvResultSet.php @@ -4,82 +4,9 @@ namespace Icinga\Module\Icingadb\Data; -use DateTime; -use DateTimeZone; use Icinga\Module\Icingadb\Redis\VolatileStateResults; -use ipl\Orm\Model; -use ipl\Orm\Query; class CsvResultSet extends VolatileStateResults { - protected $isCacheDisabled = true; - - /** - * @return array - */ - public function current(): array - { - return $this->extractKeysAndValues(parent::current()); - } - - protected function formatValue(string $key, $value): ?string - { - if ( - $value - && ( - $key === 'id' - || substr($key, -3) === '_id' - || substr($key, -3) === '.id' - || substr($key, -9) === '_checksum' - || substr($key, -4) === '_bin' - ) - ) { - $value = bin2hex($value); - } - - if (is_bool($value)) { - return $value ? 'true' : 'false'; - } elseif (is_string($value)) { - return '"' . str_replace('"', '""', $value) . '"'; - } elseif (is_array($value)) { - return '"' . implode(',', $value) . '"'; - } elseif ($value instanceof DateTime) { - return $value->setTimezone(new DateTimeZone('UTC')) - ->format('Y-m-d\TH:i:s.vP'); - } else { - return $value; - } - } - - protected function extractKeysAndValues(Model $model, string $path = ''): array - { - $keysAndValues = []; - foreach ($model as $key => $value) { - $keyPath = ($path ? $path . '.' : '') . $key; - if ($value instanceof Model) { - $keysAndValues += $this->extractKeysAndValues($value, $keyPath); - } else { - $keysAndValues[$keyPath] = $this->formatValue($key, $value); - } - } - - return $keysAndValues; - } - - public static function stream(Query $query): void - { - $query->setResultSetClass(__CLASS__); - - foreach ($query as $i => $keysAndValues) { - if ($i === 0) { - echo implode(',', array_keys($keysAndValues)); - } - - echo "\r\n"; - - echo implode(',', array_values($keysAndValues)); - } - - exit; - } + use CsvResultSetUtils; } diff --git a/library/Icingadb/Data/CsvResultSetUtils.php b/library/Icingadb/Data/CsvResultSetUtils.php new file mode 100644 index 000000000..42b25dfa5 --- /dev/null +++ b/library/Icingadb/Data/CsvResultSetUtils.php @@ -0,0 +1,82 @@ + + */ + public function current(): array + { + return $this->extractKeysAndValues(parent::current()); + } + + protected function formatValue(string $key, $value): ?string + { + if ( + $value + && ( + $key === 'id' + || substr($key, -3) === '_id' + || substr($key, -3) === '.id' + || substr($key, -9) === '_checksum' + || substr($key, -4) === '_bin' + ) + ) { + $value = bin2hex($value); + } + + if (is_bool($value)) { + return $value ? 'true' : 'false'; + } elseif (is_string($value)) { + return '"' . str_replace('"', '""', $value) . '"'; + } elseif (is_array($value)) { + return '"' . implode(',', $value) . '"'; + } elseif ($value instanceof DateTime) { + return $value->setTimezone(new DateTimeZone('UTC')) + ->format('Y-m-d\TH:i:s.vP'); + } else { + return $value; + } + } + + protected function extractKeysAndValues(Model $model, string $path = ''): array + { + $keysAndValues = []; + foreach ($model as $key => $value) { + $keyPath = ($path ? $path . '.' : '') . $key; + if ($value instanceof Model) { + $keysAndValues += $this->extractKeysAndValues($value, $keyPath); + } else { + $keysAndValues[$keyPath] = $this->formatValue($key, $value); + } + } + + return $keysAndValues; + } + + public static function stream(Query $query): void + { + $query->setResultSetClass(__CLASS__); + + foreach ($query->execute()->disableCache() as $i => $keysAndValues) { + if ($i === 0) { + echo implode(',', array_keys($keysAndValues)); + } + + echo "\r\n"; + + echo implode(',', array_values($keysAndValues)); + } + + exit; + } +} diff --git a/library/Icingadb/Data/JsonResultSet.php b/library/Icingadb/Data/JsonResultSet.php index 73cd9ef17..e5507bbf0 100644 --- a/library/Icingadb/Data/JsonResultSet.php +++ b/library/Icingadb/Data/JsonResultSet.php @@ -4,77 +4,9 @@ namespace Icinga\Module\Icingadb\Data; -use DateTime; -use DateTimeZone; use Icinga\Module\Icingadb\Redis\VolatileStateResults; -use Icinga\Util\Json; -use ipl\Orm\Model; -use ipl\Orm\Query; class JsonResultSet extends VolatileStateResults { - protected $isCacheDisabled = true; - - /** - * @return array - */ - public function current(): array - { - return $this->createObject(parent::current()); - } - - protected function formatValue(string $key, $value): ?string - { - if ( - $value - && ( - $key === 'id' - || substr($key, -3) === '_id' - || substr($key, -3) === '.id' - || substr($key, -9) === '_checksum' - || substr($key, -4) === '_bin' - ) - ) { - $value = bin2hex($value); - } - - if ($value instanceof DateTime) { - return $value->setTimezone(new DateTimeZone('UTC')) - ->format('Y-m-d\TH:i:s.vP'); - } - - return $value; - } - - protected function createObject(Model $model): array - { - $keysAndValues = []; - foreach ($model as $key => $value) { - if ($value instanceof Model) { - $keysAndValues[$key] = $this->createObject($value); - } else { - $keysAndValues[$key] = $this->formatValue($key, $value); - } - } - - return $keysAndValues; - } - - public static function stream(Query $query): void - { - $query->setResultSetClass(__CLASS__); - - echo '['; - foreach ($query as $i => $object) { - if ($i > 0) { - echo ",\n"; - } - - echo Json::sanitize($object); - } - - echo ']'; - - exit; - } + use JsonResultSetUtils; } diff --git a/library/Icingadb/Data/JsonResultSetUtils.php b/library/Icingadb/Data/JsonResultSetUtils.php new file mode 100644 index 000000000..23d04913e --- /dev/null +++ b/library/Icingadb/Data/JsonResultSetUtils.php @@ -0,0 +1,77 @@ + + */ + public function current(): array + { + return $this->createObject(parent::current()); + } + + protected function formatValue(string $key, $value) + { + if ( + $value + && ( + $key === 'id' + || substr($key, -3) === '_id' + || substr($key, -3) === '.id' + || substr($key, -9) === '_checksum' + || substr($key, -4) === '_bin' + ) + ) { + $value = bin2hex($value); + } + + if ($value instanceof DateTime) { + return $value->setTimezone(new DateTimeZone('UTC')) + ->format('Y-m-d\TH:i:s.vP'); + } + + return $value; + } + + protected function createObject(Model $model): array + { + $keysAndValues = []; + foreach ($model as $key => $value) { + if ($value instanceof Model) { + $keysAndValues[$key] = $this->createObject($value); + } else { + $keysAndValues[$key] = $this->formatValue($key, $value); + } + } + + return $keysAndValues; + } + + public static function stream(Query $query): void + { + $query->setResultSetClass(__CLASS__); + + echo '['; + foreach ($query->execute()->disableCache() as $i => $object) { + if ($i > 0) { + echo ",\n"; + } + + echo Json::sanitize($object); + } + + echo ']'; + + exit; + } +} From e311d8b9a9b6d4639c9bb3b9b02e8792e5126771 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 20 Mar 2024 15:00:03 +0100 Subject: [PATCH 2/6] Use separate result sets when exporting hosts/services --- library/Icingadb/Data/CsvResultSet.php | 4 ++-- library/Icingadb/Data/CsvResultSetUtils.php | 8 +++++++- library/Icingadb/Data/JsonResultSet.php | 4 ++-- library/Icingadb/Data/JsonResultSetUtils.php | 8 +++++++- library/Icingadb/Data/VolatileCsvResults.php | 15 +++++++++++++++ library/Icingadb/Data/VolatileJsonResults.php | 15 +++++++++++++++ 6 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 library/Icingadb/Data/VolatileCsvResults.php create mode 100644 library/Icingadb/Data/VolatileJsonResults.php diff --git a/library/Icingadb/Data/CsvResultSet.php b/library/Icingadb/Data/CsvResultSet.php index 1e6d7c546..d65364e9a 100644 --- a/library/Icingadb/Data/CsvResultSet.php +++ b/library/Icingadb/Data/CsvResultSet.php @@ -4,9 +4,9 @@ namespace Icinga\Module\Icingadb\Data; -use Icinga\Module\Icingadb\Redis\VolatileStateResults; +use ipl\Orm\ResultSet; -class CsvResultSet extends VolatileStateResults +class CsvResultSet extends ResultSet { use CsvResultSetUtils; } diff --git a/library/Icingadb/Data/CsvResultSetUtils.php b/library/Icingadb/Data/CsvResultSetUtils.php index 42b25dfa5..254341af1 100644 --- a/library/Icingadb/Data/CsvResultSetUtils.php +++ b/library/Icingadb/Data/CsvResultSetUtils.php @@ -6,6 +6,8 @@ use DateTime; use DateTimeZone; +use Icinga\Module\Icingadb\Model\Host; +use Icinga\Module\Icingadb\Model\Service; use ipl\Orm\Model; use ipl\Orm\Query; @@ -65,7 +67,11 @@ protected function extractKeysAndValues(Model $model, string $path = ''): array public static function stream(Query $query): void { - $query->setResultSetClass(__CLASS__); + if ($query->getModel() instanceof Host || $query->getModel() instanceof Service) { + $query->setResultSetClass(VolatileCsvResults::class); + } else { + $query->setResultSetClass(__CLASS__); + } foreach ($query->execute()->disableCache() as $i => $keysAndValues) { if ($i === 0) { diff --git a/library/Icingadb/Data/JsonResultSet.php b/library/Icingadb/Data/JsonResultSet.php index e5507bbf0..715437a92 100644 --- a/library/Icingadb/Data/JsonResultSet.php +++ b/library/Icingadb/Data/JsonResultSet.php @@ -4,9 +4,9 @@ namespace Icinga\Module\Icingadb\Data; -use Icinga\Module\Icingadb\Redis\VolatileStateResults; +use ipl\Orm\ResultSet; -class JsonResultSet extends VolatileStateResults +class JsonResultSet extends ResultSet { use JsonResultSetUtils; } diff --git a/library/Icingadb/Data/JsonResultSetUtils.php b/library/Icingadb/Data/JsonResultSetUtils.php index 23d04913e..b314b1e69 100644 --- a/library/Icingadb/Data/JsonResultSetUtils.php +++ b/library/Icingadb/Data/JsonResultSetUtils.php @@ -6,6 +6,8 @@ use DateTime; use DateTimeZone; +use Icinga\Module\Icingadb\Model\Host; +use Icinga\Module\Icingadb\Model\Service; use Icinga\Util\Json; use ipl\Orm\Model; use ipl\Orm\Query; @@ -59,7 +61,11 @@ protected function createObject(Model $model): array public static function stream(Query $query): void { - $query->setResultSetClass(__CLASS__); + if ($query->getModel() instanceof Host || $query->getModel() instanceof Service) { + $query->setResultSetClass(VolatileJsonResults::class); + } else { + $query->setResultSetClass(__CLASS__); + } echo '['; foreach ($query->execute()->disableCache() as $i => $object) { diff --git a/library/Icingadb/Data/VolatileCsvResults.php b/library/Icingadb/Data/VolatileCsvResults.php new file mode 100644 index 000000000..7d9f8ab35 --- /dev/null +++ b/library/Icingadb/Data/VolatileCsvResults.php @@ -0,0 +1,15 @@ + Date: Wed, 20 Mar 2024 16:34:34 +0100 Subject: [PATCH 3/6] Controller: Properly reset the default query limit during exports --- library/Icingadb/Web/Controller.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/Icingadb/Web/Controller.php b/library/Icingadb/Web/Controller.php index aa083a70a..ffa711ac8 100644 --- a/library/Icingadb/Web/Controller.php +++ b/library/Icingadb/Web/Controller.php @@ -368,7 +368,10 @@ public function export(Query ...$queries) // No matter the format, a limit should only apply if set if ($this->format !== null) { - $query->limit(Url::fromRequest()->getParam('limit')); + if (! Url::fromRequest()->hasParam('limit')) { + $query->limit(null) + ->offset(null); + } } if ($this->format === 'json' || $this->format === 'csv') { From 84735b67b7691a9e4a618fc380e8635d23634449 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 20 Mar 2024 16:36:01 +0100 Subject: [PATCH 4/6] JsonResultSetUtils: Query results in bulk.. ..if there's no limit imposed. PDO runs queries in buffered mode by default. Fetching without a limit needlessly increases the risk to require more memory than available. --- library/Icingadb/Data/JsonResultSetUtils.php | 59 ++++++++++++++++++-- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/library/Icingadb/Data/JsonResultSetUtils.php b/library/Icingadb/Data/JsonResultSetUtils.php index b314b1e69..422a5b273 100644 --- a/library/Icingadb/Data/JsonResultSetUtils.php +++ b/library/Icingadb/Data/JsonResultSetUtils.php @@ -67,17 +67,66 @@ public static function stream(Query $query): void $query->setResultSetClass(__CLASS__); } + if ($query->hasLimit()) { + // Custom limits should still apply + $query->peekAhead(false); + $offset = $query->getOffset(); + } else { + $query->limit(1000); + $query->peekAhead(); + $offset = 0; + } + echo '['; - foreach ($query->execute()->disableCache() as $i => $object) { - if ($i > 0) { - echo ",\n"; + + do { + $query->offset($offset); + $result = $query->execute()->disableCache(); + foreach ($result as $i => $object) { + if ($i > 0 || $offset !== 0) { + echo ",\n"; + } + + echo Json::sanitize($object); + + self::giveMeMoreTime(); } - echo Json::sanitize($object); - } + $offset += 1000; + } while ($result->hasMore()); echo ']'; exit; } + + /** + * Grant the caller more time to work with + * + * This resets the execution time before it runs out. The advantage of this, compared with no execution time + * limit at all, is that only the caller can bypass the limit. Any other (faulty) code will still be stopped. + * + * @internal Don't use outside of {@see JsonResultSet::stream()} or {@see CsvResultSet::stream()} + * + * @return void + */ + public static function giveMeMoreTime() + { + $spent = getrusage(); + if ($spent !== false) { + $maxExecutionTime = ini_get('max_execution_time'); + if (! $maxExecutionTime || ! is_numeric($maxExecutionTime)) { + $maxExecutionTime = 30; + } else { + $maxExecutionTime = (int) $maxExecutionTime; + } + + if ($maxExecutionTime > 0) { + $timeRemaining = $maxExecutionTime - $spent['ru_utime.tv_sec'] % $maxExecutionTime; + if ($timeRemaining <= 5) { + set_time_limit($maxExecutionTime); + } + } + } + } } From 9de4e33f942b7e6b337511f5f5fa84497a51b180 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 20 Mar 2024 16:39:31 +0100 Subject: [PATCH 5/6] CsvResultSetUtils: Query results in bulk.. ..if there's no limit imposed. PDO runs queries in buffered mode by default. Fetching without a limit needlessly increases the risk to require more memory than available. --- library/Icingadb/Data/CsvResultSetUtils.php | 32 ++++++++++++++++----- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/library/Icingadb/Data/CsvResultSetUtils.php b/library/Icingadb/Data/CsvResultSetUtils.php index 254341af1..61995d3a2 100644 --- a/library/Icingadb/Data/CsvResultSetUtils.php +++ b/library/Icingadb/Data/CsvResultSetUtils.php @@ -73,15 +73,33 @@ public static function stream(Query $query): void $query->setResultSetClass(__CLASS__); } - foreach ($query->execute()->disableCache() as $i => $keysAndValues) { - if ($i === 0) { - echo implode(',', array_keys($keysAndValues)); - } + if ($query->hasLimit()) { + // Custom limits should still apply + $query->peekAhead(false); + $offset = $query->getOffset(); + } else { + $query->limit(1000); + $query->peekAhead(); + $offset = 0; + } - echo "\r\n"; + do { + $query->offset($offset); + $result = $query->execute()->disableCache(); + foreach ($result as $i => $keysAndValues) { + if ($i === 0) { + echo implode(',', array_keys($keysAndValues)); + } - echo implode(',', array_values($keysAndValues)); - } + echo "\r\n"; + + echo implode(',', array_values($keysAndValues)); + + JsonResultSet::giveMeMoreTime(); + } + + $offset += 1000; + } while ($result->hasMore()); exit; } From 95849d961403b17a2f1e167beeb92113961f7009 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 22 Mar 2024 15:41:46 +0100 Subject: [PATCH 6/6] Update phpstan baselines --- phpstan-baseline-7x.neon | 10 +++++++ phpstan-baseline-8x.neon | 10 +++++++ phpstan-baseline-standard.neon | 55 ++++++++++++++++++++++++++++++---- 3 files changed, 70 insertions(+), 5 deletions(-) diff --git a/phpstan-baseline-7x.neon b/phpstan-baseline-7x.neon index 3bcc153a0..e8d2562f2 100644 --- a/phpstan-baseline-7x.neon +++ b/phpstan-baseline-7x.neon @@ -105,6 +105,16 @@ parameters: count: 1 path: library/Icingadb/Data/PivotTable.php + - + message: "#^Parameter \\#1 \\$input of function array_keys expects array, mixed given\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileCsvResults.php + + - + message: "#^Parameter \\#1 \\$input of function array_values expects array, mixed given\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileCsvResults.php + - message: "#^Parameter \\#2 \\$str of function explode expects string, mixed given\\.$#" count: 1 diff --git a/phpstan-baseline-8x.neon b/phpstan-baseline-8x.neon index b472980a2..2cea59701 100644 --- a/phpstan-baseline-8x.neon +++ b/phpstan-baseline-8x.neon @@ -105,6 +105,16 @@ parameters: count: 1 path: library/Icingadb/Data/PivotTable.php + - + message: "#^Parameter \\#1 \\$array of function array_keys expects array, mixed given\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileCsvResults.php + + - + message: "#^Parameter \\#1 \\$array of function array_values expects array, mixed given\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileCsvResults.php + - message: "#^Parameter \\#2 \\$string of function explode expects string, mixed given\\.$#" count: 1 diff --git a/phpstan-baseline-standard.neon b/phpstan-baseline-standard.neon index f91819bc7..70fd385d2 100644 --- a/phpstan-baseline-standard.neon +++ b/phpstan-baseline-standard.neon @@ -2735,6 +2735,11 @@ parameters: count: 1 path: library/Icingadb/Data/JsonResultSet.php + - + message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\JsonResultSet\\:\\:formatValue\\(\\) has no return type specified\\.$#" + count: 1 + path: library/Icingadb/Data/JsonResultSet.php + - message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\JsonResultSet\\:\\:formatValue\\(\\) has parameter \\$value with no type specified\\.$#" count: 1 @@ -2790,6 +2795,51 @@ parameters: count: 1 path: library/Icingadb/Data/PivotTable.php + - + message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\VolatileCsvResults\\:\\:extractKeysAndValues\\(\\) return type has no value type specified in iterable type array\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileCsvResults.php + + - + message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\VolatileCsvResults\\:\\:formatValue\\(\\) has parameter \\$value with no type specified\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileCsvResults.php + + - + message: "#^Parameter \\#1 \\$key of method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\VolatileCsvResults\\:\\:formatValue\\(\\) expects string, mixed given\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileCsvResults.php + + - + message: "#^Parameter \\#1 \\$model of method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\VolatileCsvResults\\:\\:extractKeysAndValues\\(\\) expects ipl\\\\Orm\\\\Model, mixed given\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileCsvResults.php + + - + message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\VolatileJsonResults\\:\\:createObject\\(\\) return type has no value type specified in iterable type array\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileJsonResults.php + + - + message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\VolatileJsonResults\\:\\:formatValue\\(\\) has no return type specified\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileJsonResults.php + + - + message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\VolatileJsonResults\\:\\:formatValue\\(\\) has parameter \\$value with no type specified\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileJsonResults.php + + - + message: "#^Parameter \\#1 \\$key of method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\VolatileJsonResults\\:\\:formatValue\\(\\) expects string, mixed given\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileJsonResults.php + + - + message: "#^Parameter \\#1 \\$model of method Icinga\\\\Module\\\\Icingadb\\\\Data\\\\VolatileJsonResults\\:\\:createObject\\(\\) expects ipl\\\\Orm\\\\Model, mixed given\\.$#" + count: 1 + path: library/Icingadb/Data/VolatileJsonResults.php + - message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Hook\\\\ActionsHook\\\\ObjectActionsHook\\:\\:init\\(\\) has no return type specified\\.$#" count: 1 @@ -5405,11 +5455,6 @@ parameters: count: 1 path: library/Icingadb/Web/Controller.php - - - message: "#^Parameter \\#1 \\$limit of method ipl\\\\Orm\\\\Query\\:\\:limit\\(\\) expects int\\|null, mixed given\\.$#" - count: 1 - path: library/Icingadb/Web/Controller.php - - message: "#^Parameter \\#1 \\$queryString of method Icinga\\\\Module\\\\Icingadb\\\\Web\\\\Controller\\:\\:parseRestriction\\(\\) expects string, array\\ given\\.$#" count: 3