From eb1429a4a1015fee814a208df7033d00ddd83143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Anne?= Date: Tue, 26 Nov 2024 09:16:41 +0100 Subject: [PATCH 1/2] PHP 8.4 compatibility --- .github/workflows/ci.yml | 14 ++++---- composer.json | 18 +++++----- composer.lock | 31 +++++++++------- public/index.php | 4 +-- src/Consumable.php | 2 +- src/Glpi/Asset/Asset_PeripheralAsset.php | 2 +- src/Glpi/DBAL/QueryFunction.php | 6 ++-- src/Glpi/Dashboard/FakeProvider.php | 6 ++-- src/Glpi/Features/State.php | 5 ++- .../ServiceCatalog/ServiceCatalogManager.php | 2 +- src/Glpi/Marketplace/Controller.php | 4 +-- src/Glpi/System/Log/LogParser.php | 2 +- src/IPAddress.php | 6 ++-- src/Line.php | 2 +- src/QueuedWebhook.php | 4 +-- src/Reservation.php | 2 +- src/Toolbox.php | 2 +- src/autoload/constants.php | 2 +- tests/src/FormTesterTrait.php | 4 +-- tools/patches/apereo-phpcas-php84.patch | 35 +++++++++++++++++++ 20 files changed, 97 insertions(+), 56 deletions(-) create mode 100644 tools/patches/apereo-phpcas-php84.patch diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5e8f27d3267..4c3ee2342fa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,7 +35,7 @@ jobs: matrix: include: - {php-version: "8.2"} # Lint on lower PHP version to detected too early usage of new syntaxes - - {php-version: "8.3"} # Lint on higher PHP version to detected deprecated elements usage + - {php-version: "8.4"} # Lint on higher PHP version to detected deprecated elements usage env: COMPOSE_FILE: ".github/actions/docker-compose-app.yml" APPLICATION_ROOT: "${{ github.workspace }}" @@ -116,11 +116,11 @@ jobs: MATRIX=' { "include": [ - {"php-version": "8.3", "db-image": "mariadb:11.4"}, - {"php-version": "8.3", "db-image": "mysql:8.4"}, + {"php-version": "8.4", "db-image": "mariadb:11.4"}, + {"php-version": "8.4", "db-image": "mysql:8.4"}, {"php-version": "8.2", "db-image": "mariadb:11.4"}, - {"php-version": "8.3", "db-image": "mariadb:10.5"}, - {"php-version": "8.3", "db-image": "percona:8.0"} + {"php-version": "8.4", "db-image": "mariadb:10.5"}, + {"php-version": "8.4", "db-image": "percona:8.0"} ] } ' @@ -128,8 +128,8 @@ jobs: MATRIX=' { "include": [ - {"php-version": "8.3", "db-image": "mariadb:11.4"}, - {"php-version": "8.3", "db-image": "mysql:8.4"} + {"php-version": "8.4", "db-image": "mariadb:11.4"}, + {"php-version": "8.4", "db-image": "mysql:8.4"} ] } ' diff --git a/composer.json b/composer.json index 4abf6cdba5c..53d9cc87528 100644 --- a/composer.json +++ b/composer.json @@ -104,8 +104,8 @@ }, "require-dev": { "ext-xml": "*", - "atoum/atoum": "^4.2", - "atoum/stubs": "^2.6", + "atoum/atoum": "dev-main#e43a6a84809635cf5dcdf80bf3c773e7cf1d5a04", + "atoum/stubs": "dev-master#fb8890f347a7ecf1e3cf2b5a139a8b038359a6ab", "friendsoftwig/twigcs": "^6.4", "glpi-project/tools": "^0.7", "mikey179/vfsstream": "^1.6", @@ -188,19 +188,21 @@ "post-install-cmd": [ "@php -r \"file_put_contents('.composer.hash', sha1_file('composer.lock'));\"", "@php -f vendor/bin/build_hw_jsons", - "patch -f -p1 -d vendor/laminas/laminas-mail/ < tools/patches/laminas-mail-invalid-header-ignore.patch || true", - "patch -f -p1 -d vendor/laminas/laminas-mail/ < tools/patches/laminas-mail-address-no-length-check.patch || true", - "patch -f -p1 -d vendor/guzzlehttp/guzzle/ < tools/patches/guzzle-http-client-restrict-http-methods.patch || true" + "@patch" ], "post-update-cmd": [ "@php -r \"file_put_contents('.composer.hash', sha1_file('composer.lock'));\"", "@php -f vendor/bin/build_hw_jsons", - "patch -f -p1 -d vendor/laminas/laminas-mail/ < tools/patches/laminas-mail-invalid-header-ignore.patch || true", - "patch -f -p1 -d vendor/laminas/laminas-mail/ < tools/patches/laminas-mail-address-no-length-check.patch || true", - "patch -f -p1 -d vendor/guzzlehttp/guzzle/ < tools/patches/guzzle-http-client-restrict-http-methods.patch || true" + "@patch" ], "build": [ "bin/console dependencies install" + ], + "patch": [ + "patch -f -p1 -d vendor/laminas/laminas-mail/ < tools/patches/laminas-mail-invalid-header-ignore.patch || true", + "patch -f -p1 -d vendor/laminas/laminas-mail/ < tools/patches/laminas-mail-address-no-length-check.patch || true", + "patch -f -p1 -d vendor/guzzlehttp/guzzle/ < tools/patches/guzzle-http-client-restrict-http-methods.patch || true", + "patch -f -p1 -d vendor/apereo/phpcas/ < tools/patches/apereo-phpcas-php84.patch || true" ] } } diff --git a/composer.lock b/composer.lock index d40b9cab3f1..70515fb32a8 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "f7fbe4971a89637e80066017f3eeef72", + "content-hash": "093f173c565f7d2b5be80c807843fc1a", "packages": [ { "name": "apereo/phpcas", @@ -8986,16 +8986,16 @@ "packages-dev": [ { "name": "atoum/atoum", - "version": "4.2.0", + "version": "dev-main", "source": { "type": "git", "url": "https://github.com/atoum/atoum.git", - "reference": "fd9a339de252ea774ab755b3fd25dd19be2dbefb" + "reference": "e43a6a84809635cf5dcdf80bf3c773e7cf1d5a04" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/atoum/atoum/zipball/fd9a339de252ea774ab755b3fd25dd19be2dbefb", - "reference": "fd9a339de252ea774ab755b3fd25dd19be2dbefb", + "url": "https://api.github.com/repos/atoum/atoum/zipball/e43a6a84809635cf5dcdf80bf3c773e7cf1d5a04", + "reference": "e43a6a84809635cf5dcdf80bf3c773e7cf1d5a04", "shasum": "" }, "require": { @@ -9016,6 +9016,7 @@ "ext-mbstring": "Provides support for UTF-8 strings", "ext-xdebug": "Provides code coverage report (>= 2.3)" }, + "default-branch": true, "bin": [ "bin/atoum" ], @@ -9067,27 +9068,28 @@ ], "support": { "issues": "https://github.com/atoum/atoum/issues", - "source": "https://github.com/atoum/atoum/tree/4.2.0" + "source": "https://github.com/atoum/atoum/tree/main" }, - "time": "2023-07-30T12:52:23+00:00" + "time": "2024-07-31T12:02:10+00:00" }, { "name": "atoum/stubs", - "version": "2.6.0", + "version": "dev-master", "source": { "type": "git", "url": "https://github.com/atoum/stubs.git", - "reference": "df8b73b0358de7283ecba91d8f4a9683f583993d" + "reference": "fb8890f347a7ecf1e3cf2b5a139a8b038359a6ab" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/atoum/stubs/zipball/df8b73b0358de7283ecba91d8f4a9683f583993d", - "reference": "df8b73b0358de7283ecba91d8f4a9683f583993d", + "url": "https://api.github.com/repos/atoum/stubs/zipball/fb8890f347a7ecf1e3cf2b5a139a8b038359a6ab", + "reference": "fb8890f347a7ecf1e3cf2b5a139a8b038359a6ab", "shasum": "" }, "suggest": { "atoum/atoum": "Include atoum in your projet dependencies" }, + "default-branch": true, "type": "library", "extra": { "branch-alias": { @@ -9120,7 +9122,7 @@ "issues": "https://github.com/atoum/stubs/issues", "source": "https://github.com/atoum/stubs/tree/master" }, - "time": "2018-01-29T22:41:37+00:00" + "time": "2024-07-30T18:02:37+00:00" }, { "name": "friendsoftwig/twigcs", @@ -11723,7 +11725,10 @@ ], "aliases": [], "minimum-stability": "stable", - "stability-flags": [], + "stability-flags": { + "atoum/atoum": 20, + "atoum/stubs": 20 + }, "prefer-stable": false, "prefer-lowest": false, "platform": { diff --git a/public/index.php b/public/index.php index e0f2b05cc1a..c5fe79be48a 100755 --- a/public/index.php +++ b/public/index.php @@ -37,8 +37,8 @@ // Check PHP version not to have trouble // Need to be the very fist step before any include -if (version_compare(PHP_VERSION, '8.2.0', '<') || version_compare(PHP_VERSION, '8.3.999', '>')) { - exit('PHP version must be between 8.2 and 8.3.'); +if (version_compare(PHP_VERSION, '8.2.0', '<') || version_compare(PHP_VERSION, '8.4.999', '>')) { + exit('PHP version must be between 8.2 and 8.4.'); } // Check the resources state before trying to instanciate the Kernel. diff --git a/src/Consumable.php b/src/Consumable.php index 54d83e93e2f..2e04a699499 100644 --- a/src/Consumable.php +++ b/src/Consumable.php @@ -197,7 +197,7 @@ public static function getMassiveActionsForItemtype( array &$actions, $itemtype, $is_deleted = 0, - CommonDBTM $checkitem = null + ?CommonDBTM $checkitem = null ) { // Special actions only for self if ($itemtype !== static::class) { diff --git a/src/Glpi/Asset/Asset_PeripheralAsset.php b/src/Glpi/Asset/Asset_PeripheralAsset.php index 50a907a8431..50f8f30ad23 100644 --- a/src/Glpi/Asset/Asset_PeripheralAsset.php +++ b/src/Glpi/Asset/Asset_PeripheralAsset.php @@ -258,7 +258,7 @@ public static function getMassiveActionsForItemtype( array &$actions, $itemtype, $is_deleted = false, - CommonDBTM $checkitem = null + ?CommonDBTM $checkitem = null ) { $action_prefix = __CLASS__ . MassiveAction::CLASS_ACTION_SEPARATOR; $specificities = self::getRelationMassiveActionsSpecificities(); diff --git a/src/Glpi/DBAL/QueryFunction.php b/src/Glpi/DBAL/QueryFunction.php index 0e0ddf61ccf..6703e4bbbbc 100644 --- a/src/Glpi/DBAL/QueryFunction.php +++ b/src/Glpi/DBAL/QueryFunction.php @@ -175,7 +175,7 @@ public static function ifnull(string|QueryExpression $expression, string|QueryEx * @param string|null $alias Function result alias (will be automatically quoted) * @return QueryExpression */ - public static function groupConcat(string|QueryExpression $expression, ?string $separator = null, bool $distinct = false, array|string $order_by = null, ?string $alias = null): QueryExpression + public static function groupConcat(string|QueryExpression $expression, ?string $separator = null, bool $distinct = false, array|string|null $order_by = null, ?string $alias = null): QueryExpression { /** @var \DBmysql $DB */ global $DB; @@ -309,7 +309,7 @@ public static function replace(string|QueryExpression $expression, string|QueryE * @param string|null $alias Function result alias (will be automatically quoted) * @return QueryExpression */ - public static function fromUnixtime(string|QueryExpression $expression, string|QueryExpression $format = null, ?string $alias = null): QueryExpression + public static function fromUnixtime(string|QueryExpression $expression, string|QueryExpression|null $format = null, ?string $alias = null): QueryExpression { $params = [$expression]; if ($format !== null) { @@ -433,7 +433,7 @@ public static function timediff(string|QueryExpression $expression1, string|Quer * @param string|null $alias Function result alias (will be automatically quoted) * @return QueryExpression */ - public static function unixTimestamp(string|QueryExpression $expression = null, ?string $alias = null): QueryExpression + public static function unixTimestamp(string|QueryExpression|null $expression = null, ?string $alias = null): QueryExpression { $params = []; if ($expression !== null) { diff --git a/src/Glpi/Dashboard/FakeProvider.php b/src/Glpi/Dashboard/FakeProvider.php index 398787c9180..c70a5ddb971 100644 --- a/src/Glpi/Dashboard/FakeProvider.php +++ b/src/Glpi/Dashboard/FakeProvider.php @@ -125,7 +125,7 @@ private static function getItemCount(?string $itemtype = null) return $values[$itemtype] ?? null; } - public static function bigNumberItem(CommonDBTM $item = null, array $params = []): array + public static function bigNumberItem(?CommonDBTM $item = null, array $params = []): array { return [ 'number' => self::getItemCount($item::class) ?? 1500, @@ -278,7 +278,7 @@ public static function nbTicketsByAgreementStatusAndTechnicianGroup(array $param ]; } - public static function nbItemByFk(CommonDBTM $item = null, CommonDBTM $fk_item = null, array $params = []): array + public static function nbItemByFk(?CommonDBTM $item = null, ?CommonDBTM $fk_item = null, array $params = []): array { $item_counts = self::getItemCount(); $number_fk = $item_counts[$fk_item::class] ?? 20; @@ -306,7 +306,7 @@ public static function nbItemByFk(CommonDBTM $item = null, CommonDBTM $fk_item = ]; } - public static function articleListItem(CommonDBTM $item = null, array $params = []): array + public static function articleListItem(?CommonDBTM $item = null, array $params = []): array { $data = []; for ($i = 0; $i < 5; $i++) { diff --git a/src/Glpi/Features/State.php b/src/Glpi/Features/State.php index ede86e18a26..88776e4fb09 100644 --- a/src/Glpi/Features/State.php +++ b/src/Glpi/Features/State.php @@ -50,12 +50,11 @@ private function checkSetup(): void global $CFG_GLPI; if (!in_array(static::class, $CFG_GLPI['state_types'])) { - trigger_error( + throw new \LogicException( sprintf( 'Class %s must be present in $CFG_GLPI[\'state_types\']', static::class - ), - E_USER_ERROR + ) ); } } diff --git a/src/Glpi/Form/ServiceCatalog/ServiceCatalogManager.php b/src/Glpi/Form/ServiceCatalog/ServiceCatalogManager.php index 9b6f33da582..a88453c0976 100644 --- a/src/Glpi/Form/ServiceCatalog/ServiceCatalogManager.php +++ b/src/Glpi/Form/ServiceCatalog/ServiceCatalogManager.php @@ -161,7 +161,7 @@ function (ServiceCatalogItemInterface $item) use ($item_request) { private function hasChildren( ItemRequest $item_request, - ServiceCatalogCompositeInterface $composite = null, + ?ServiceCatalogCompositeInterface $composite = null, ): bool { $leaf_providers = array_filter( $this->providers, diff --git a/src/Glpi/Marketplace/Controller.php b/src/Glpi/Marketplace/Controller.php index ccae020c033..a4e9b532fec 100644 --- a/src/Glpi/Marketplace/Controller.php +++ b/src/Glpi/Marketplace/Controller.php @@ -159,7 +159,7 @@ private static function getPluginVersionInfo(array $plugin, string $version): ?a * @param ?string $version Download a specific version of the plugin * @return bool */ - public function downloadPlugin($auto_install = true, string $version = null): bool + public function downloadPlugin($auto_install = true, ?string $version = null): bool { if (!self::hasWriteAccess()) { return false; @@ -464,7 +464,7 @@ public function getRequiredOffers(): array * * @return bool */ - public function canBeDownloaded(string $version = null) + public function canBeDownloaded(?string $version = null) { $api = self::getAPI(); $api_plugin = $api->getPlugin($this->plugin_key); diff --git a/src/Glpi/System/Log/LogParser.php b/src/Glpi/System/Log/LogParser.php index afb3fc4af5b..fff81594eb9 100644 --- a/src/Glpi/System/Log/LogParser.php +++ b/src/Glpi/System/Log/LogParser.php @@ -124,7 +124,7 @@ public function getLogFileInfo(string $filepath): ?array * * @return array|null */ - public function parseLogFile(string $filepath, int $max_nb_lines = null): ?array + public function parseLogFile(string $filepath, ?int $max_nb_lines = null): ?array { /** @var array $CFG_GLPI */ global $CFG_GLPI; diff --git a/src/IPAddress.php b/src/IPAddress.php index a02d3af9b5f..9439743ff25 100644 --- a/src/IPAddress.php +++ b/src/IPAddress.php @@ -1276,9 +1276,9 @@ private static function getCriteriaLinkedToNetwork(IPNetwork $network): array * @param array $options **/ public static function getHTMLTableCellsForItem( - HTMLTableRow $row = null, - CommonDBTM $item = null, - HTMLTableCell $father = null, + ?HTMLTableRow $row = null, + ?CommonDBTM $item = null, + ?HTMLTableCell $father = null, array $options = [] ) { /** @var \DBmysql $DB */ diff --git a/src/Line.php b/src/Line.php index 0915e05bbcd..ebf085e5df7 100644 --- a/src/Line.php +++ b/src/Line.php @@ -246,7 +246,7 @@ public static function getIcon() return "ti ti-phone-calling"; } - public static function getMassiveActionsForItemtype(array &$actions, $itemtype, $is_deleted = 0, CommonDBTM $checkitem = null) + public static function getMassiveActionsForItemtype(array &$actions, $itemtype, $is_deleted = 0, ?CommonDBTM $checkitem = null) { /** @var array $CFG_GLPI */ global $CFG_GLPI; diff --git a/src/QueuedWebhook.php b/src/QueuedWebhook.php index 466c20e9250..91e1bcfa481 100644 --- a/src/QueuedWebhook.php +++ b/src/QueuedWebhook.php @@ -563,7 +563,7 @@ public static function getPendings($send_time = null, $limit = 20, $extra_where * * @return integer either 0 or 1 **/ - public static function cronQueuedWebhook(CronTask $task = null) + public static function cronQueuedWebhook(?CronTask $task = null) { $cron_status = 0; @@ -589,7 +589,7 @@ public static function cronQueuedWebhook(CronTask $task = null) * * @return integer either 0 or 1 **/ - public static function cronQueuedWebhookClean(CronTask $task = null) + public static function cronQueuedWebhookClean(?CronTask $task = null) { /** @var \DBmysql $DB */ global $DB; diff --git a/src/Reservation.php b/src/Reservation.php index be3fcccef7d..6642529d841 100644 --- a/src/Reservation.php +++ b/src/Reservation.php @@ -1153,7 +1153,7 @@ public static function getIcon() return "ti ti-calendar-event"; } - public static function getMassiveActionsForItemtype(array &$actions, $itemtype, $is_deleted = 0, CommonDBTM $checkitem = null) + public static function getMassiveActionsForItemtype(array &$actions, $itemtype, $is_deleted = 0, ?CommonDBTM $checkitem = null) { /** @var array $CFG_GLPI */ global $CFG_GLPI; diff --git a/src/Toolbox.php b/src/Toolbox.php index 9b2c57bcbcf..24abda6985f 100644 --- a/src/Toolbox.php +++ b/src/Toolbox.php @@ -262,7 +262,7 @@ public static function decodeFromUtf8($string, $to_charset = "ISO-8859-1") * * @return void **/ - private static function log(LoggerInterface $logger = null, $level = LogLevel::WARNING, $args = null) + private static function log(?LoggerInterface $logger = null, $level = LogLevel::WARNING, $args = null) { static $tps = 0; diff --git a/src/autoload/constants.php b/src/autoload/constants.php index e8ec0791909..d737740f34e 100644 --- a/src/autoload/constants.php +++ b/src/autoload/constants.php @@ -45,7 +45,7 @@ ); define('GLPI_MIN_PHP', '8.2'); // Must also be changed in top of public/index.php -define('GLPI_MAX_PHP', '8.3'); // Must also be changed in top of public/index.php +define('GLPI_MAX_PHP', '8.4'); // Must also be changed in top of public/index.php define('GLPI_YEAR', '2024'); //Define a global recipient address for email notifications diff --git a/tests/src/FormTesterTrait.php b/tests/src/FormTesterTrait.php index 053a8110366..00ee9a0c36d 100644 --- a/tests/src/FormTesterTrait.php +++ b/tests/src/FormTesterTrait.php @@ -161,7 +161,7 @@ protected function createForm(FormBuilder $builder): Form protected function getQuestionId( Form $form, string $question_name, - string $section_name = null, + ?string $section_name = null, ): int { // Make sure form is up to date $form->getFromDB($form->getID()); @@ -239,7 +239,7 @@ protected function getSectionId( protected function getCommentId( Form $form, string $comment_name, - string $section_name = null, + ?string $section_name = null, ): int { // Make sure form is up to date $form->getFromDB($form->getID()); diff --git a/tools/patches/apereo-phpcas-php84.patch b/tools/patches/apereo-phpcas-php84.patch new file mode 100644 index 00000000000..6df6e7d594c --- /dev/null +++ b/tools/patches/apereo-phpcas-php84.patch @@ -0,0 +1,35 @@ +diff --git a/source/CAS.php b/source/CAS.php +index df6bc82..da145bc 100644 +--- a/source/CAS.php ++++ b/source/CAS.php +@@ -347,7 +347,7 @@ class phpCAS + */ + public static function client($server_version, $server_hostname, + $server_port, $server_uri, $service_base_url, +- $changeSessionID = true, \SessionHandlerInterface $sessionHandler = null ++ $changeSessionID = true, ?\SessionHandlerInterface $sessionHandler = null + ) { + phpCAS :: traceBegin(); + if (is_object(self::$_PHPCAS_CLIENT)) { +@@ -402,7 +402,7 @@ class phpCAS + */ + public static function proxy($server_version, $server_hostname, + $server_port, $server_uri, $service_base_url, +- $changeSessionID = true, \SessionHandlerInterface $sessionHandler = null ++ $changeSessionID = true, ?\SessionHandlerInterface $sessionHandler = null + ) { + phpCAS :: traceBegin(); + if (is_object(self::$_PHPCAS_CLIENT)) { +diff --git a/source/CAS/Client.php b/source/CAS/Client.php +index 8ca9711..42709e9 100644 +--- a/source/CAS/Client.php ++++ b/source/CAS/Client.php +@@ -938,7 +938,7 @@ class CAS_Client + $server_uri, + $service_base_url, + $changeSessionID = true, +- \SessionHandlerInterface $sessionHandler = null ++ ?\SessionHandlerInterface $sessionHandler = null + ) { + // Argument validation + if (gettype($server_version) != 'string') From 197778899b759d50bd53d7831385068b6e6270e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Anne?= Date: Tue, 26 Nov 2024 10:24:08 +0100 Subject: [PATCH 2/2] Prevent vendor deprecations to break the test suite --- phpunit/functional/TicketTest.php | 3 + phpunit/functional/ToolboxTest.php | 12 +++ src/Glpi/Application/ErrorHandler.php | 44 ++++++---- tests/functional/DBmysqlIterator.php | 6 ++ .../Glpi/Application/ErrorHandler.php | 80 ++++++++++--------- .../Glpi/Http/LegacyAssetsListener.php | 2 + .../Glpi/Http/LegacyRouterListener.php | 2 + 7 files changed, 96 insertions(+), 53 deletions(-) diff --git a/phpunit/functional/TicketTest.php b/phpunit/functional/TicketTest.php index 25a1ac4e76d..6196a7b5c1d 100644 --- a/phpunit/functional/TicketTest.php +++ b/phpunit/functional/TicketTest.php @@ -7579,7 +7579,10 @@ public function testDynamicProperties(): void { $ticket = new \Ticket(); + $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations $ticket->plugin_xxx_data = 'test'; + \error_reporting($reporting_level); // restore previous level + $this->hasPhpLogRecordThatContains( 'Creation of dynamic property Ticket::$plugin_xxx_data is deprecated', LogLevel::INFO diff --git a/phpunit/functional/ToolboxTest.php b/phpunit/functional/ToolboxTest.php index 7f9225cbb01..4b82fd36da0 100644 --- a/phpunit/functional/ToolboxTest.php +++ b/phpunit/functional/ToolboxTest.php @@ -1080,7 +1080,10 @@ public function testIsValidWebUrl(string $url, bool $result) public function testDeprecated() { + $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations \Toolbox::deprecated('Calling this function is deprecated'); + \error_reporting($reporting_level); // restore previous level + $this->hasPhpLogRecordThatContains( 'Calling this function is deprecated', LogLevel::INFO @@ -1090,7 +1093,10 @@ public function testDeprecated() public function testDeprecatedPast() { // Test planned deprecation in the past + $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations \Toolbox::deprecated('Calling this function is deprecated', true, '10.0'); + \error_reporting($reporting_level); // restore previous level + $this->hasPhpLogRecordThatContains( 'Calling this function is deprecated', LogLevel::INFO @@ -1100,7 +1106,10 @@ public function testDeprecatedPast() public function testDeprecatedCurrent() { // Test planned deprecation in current version + $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations \Toolbox::deprecated('Calling this function is deprecated', true, GLPI_VERSION); + \error_reporting($reporting_level); // restore previous level + $this->hasPhpLogRecordThatContains( 'Calling this function is deprecated', LogLevel::INFO @@ -1110,7 +1119,10 @@ public function testDeprecatedCurrent() public function testFutureDeprecated() { // Test planned deprecation in the future does NOT throw an error + $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations \Toolbox::deprecated('Calling this function is deprecated', true, '99.0'); + \error_reporting($reporting_level); // restore previous level + $this->assertTrue(true); //non empty test } diff --git a/src/Glpi/Application/ErrorHandler.php b/src/Glpi/Application/ErrorHandler.php index 0d083588502..9a3914bd6bd 100644 --- a/src/Glpi/Application/ErrorHandler.php +++ b/src/Glpi/Application/ErrorHandler.php @@ -63,7 +63,7 @@ class ErrorHandler E_USER_ERROR => LogLevel::ERROR, E_USER_WARNING => LogLevel::WARNING, E_USER_NOTICE => LogLevel::NOTICE, - E_STRICT => LogLevel::NOTICE, + 2048 => LogLevel::NOTICE, // 2048 = deprecated E_STRICT E_RECOVERABLE_ERROR => LogLevel::ERROR, E_DEPRECATED => LogLevel::INFO, E_USER_DEPRECATED => LogLevel::INFO, @@ -227,9 +227,34 @@ public function register(): void set_exception_handler([$this, 'handleException']); } - // Force reporting of all errors, to ensure that all log levels that are supposed to be - // pushed in logs according to `GLPI_LOG_LVL`/`GLPI_ENVIRONMENT_TYPE` are actually reported. - error_reporting(E_ALL); + // Adjust reporting level to the environment, to ensure that all the errors supposed to be logged are + // actually reported, and to prevent reporting other errors. + $reporting_level = E_ALL; + foreach (self::ERROR_LEVEL_MAP as $value => $log_level) { + if ( + $this->env !== GLPI::ENV_DEVELOPMENT + && in_array($log_level, [LogLevel::DEBUG, LogLevel::INFO]) + ) { + // Do not report debug and info messages unless in development env. + // Suppressing the INFO level will prevent deprecations to be pushed in other environments logs. + // + // Suppressing the deprecations in the testing environment is mandatory to prevent deprecations + // triggered in vendor code to make our test suite fail. + // We may review this part once we will have migrate all our test suite on PHPUnit. + // For now, we rely on PHPStan to detect usages of deprecated code. + $reporting_level = $reporting_level & ~$value; + } + + if ( + !in_array($this->env, [GLPI::ENV_DEVELOPMENT, GLPI::ENV_TESTING], true) + && $log_level === LogLevel::NOTICE + ) { + // Do not report notice messages unless in development/testing env. + // Notices are errors with no functional impact, so we do not want people to report them as issues. + $reporting_level = $reporting_level & ~$value; + } + } + error_reporting($reporting_level); // Disable native error displaying as it will be handled by `self::outputDebugMessage()`. ini_set('display_errors', 'Off'); @@ -435,15 +460,6 @@ private function outputDebugMessage(string $error_type, string $message, string return; } - if ( - in_array($this->env, [GLPI::ENV_STAGING, GLPI::ENV_PRODUCTION]) - && in_array($log_level, [LogLevel::DEBUG, LogLevel::INFO]) - ) { - // On staging/production environment, do not output debug/info messages. - // These messages are not supposed to be intended for end users, as they have no functional impact. - return; - } - $is_dev_env = $this->env === GLPI::ENV_DEVELOPMENT; $is_debug_mode = isset($_SESSION['glpi_use_mode']) && $_SESSION['glpi_use_mode'] == \Session::DEBUG_MODE; $is_console_context = $this->output_handler instanceof OutputInterface; @@ -513,7 +529,7 @@ private function codeToString(int $error_code): string E_USER_ERROR => 'User Error', E_USER_WARNING => 'User Warning', E_USER_NOTICE => 'User Notice', - E_STRICT => 'Runtime Notice', + 2048 => 'Runtime Notice', // 2048 = deprecated E_STRICT E_RECOVERABLE_ERROR => 'Catchable Fatal Error', E_DEPRECATED => 'Deprecated function', E_USER_DEPRECATED => 'User deprecated function', diff --git a/tests/functional/DBmysqlIterator.php b/tests/functional/DBmysqlIterator.php index a7dde31ee69..ad9715e904b 100644 --- a/tests/functional/DBmysqlIterator.php +++ b/tests/functional/DBmysqlIterator.php @@ -1635,7 +1635,9 @@ public function testConvertOldRequestArgsToCriteria(array $params, array $expect $result = null; $this->when( function () use ($iterator, $params, &$result) { + $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations $result = $this->callPrivateMethod($iterator, 'convertOldRequestArgsToCriteria', $params, 'test'); + \error_reporting($reporting_level); // restore previous level } ) ->error @@ -1665,7 +1667,9 @@ public function testExecuteWithOldSignature(array $params, array $expected, stri $iterator = new \DBmysqlIterator($db); $this->when( function () use ($iterator, $params) { + $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations $iterator = $iterator->execute(...$params); + \error_reporting($reporting_level); // restore previous level } ) ->error @@ -1701,7 +1705,9 @@ public function testBuildQueryWithOldSignature(array $params, array $expected, s $iterator = new \DBmysqlIterator($db); $this->when( function () use ($iterator, $params) { + $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations $iterator = $iterator->buildQuery(...$params); + \error_reporting($reporting_level); // restore previous level } ) ->error diff --git a/tests/functional/Glpi/Application/ErrorHandler.php b/tests/functional/Glpi/Application/ErrorHandler.php index d59840bf776..ad35b5c065b 100644 --- a/tests/functional/Glpi/Application/ErrorHandler.php +++ b/tests/functional/Glpi/Application/ErrorHandler.php @@ -52,6 +52,9 @@ protected function errorProvider(): iterable foreach ([Session::NORMAL_MODE, Session::DEBUG_MODE] as $session_mode) { foreach ([GLPI::ENV_DEVELOPMENT, GLPI::ENV_TESTING, GLPI::ENV_STAGING, GLPI::ENV_PRODUCTION] as $env) { + $suppress_info = $env !== GLPI::ENV_DEVELOPMENT; + $suppress_notice = !in_array($env, [GLPI::ENV_DEVELOPMENT, GLPI::ENV_TESTING], true); + yield [ 'session_mode' => $session_mode, 'env' => $env, @@ -82,8 +85,8 @@ protected function errorProvider(): iterable 'error_call' => function () { trigger_error('some notice', E_USER_NOTICE); }, - 'expected_log_level' => Level::Notice, - 'expected_msg_pattern' => $log_prefix + 'expected_log_level' => $suppress_notice ? null : Level::Notice, + 'expected_msg_pattern' => $suppress_notice ? null : $log_prefix . preg_quote('PHP User Notice (' . E_USER_NOTICE . '): some notice', '/') . $log_suffix, ]; @@ -94,8 +97,8 @@ protected function errorProvider(): iterable 'error_call' => function () { trigger_error('this method is deprecated', E_USER_DEPRECATED); }, - 'expected_log_level' => Level::Info, - 'expected_msg_pattern' => $log_prefix + 'expected_log_level' => $suppress_info ? null : Level::Info, + 'expected_msg_pattern' => $suppress_info ? null : $log_prefix . preg_quote('PHP User deprecated function (' . E_USER_DEPRECATED . '): this method is deprecated', '/') . $log_suffix, ]; @@ -107,8 +110,8 @@ protected function errorProvider(): iterable $param = new \ReflectionParameter([\Config::class, 'getTypeName'], 0); $param->isCallable(); }, - 'expected_log_level' => Level::Info, - 'expected_msg_pattern' => $log_prefix + 'expected_log_level' => $suppress_info ? null : Level::Info, + 'expected_msg_pattern' => $suppress_info ? null : $log_prefix . preg_quote('PHP Deprecated function (' . E_DEPRECATED . '): Method ReflectionParameter::isCallable() is deprecated', '/') . $log_suffix, ]; @@ -128,13 +131,13 @@ public function testRegisteredErrorHandler( int $session_mode, string $env, callable $error_call, - Level $expected_log_level, - string $expected_msg_pattern + ?Level $expected_log_level, + ?string $expected_msg_pattern ) { $handler = new TestHandler(); $logger = $this->newMockInstance('Monolog\\Logger', null, null, ['test-logger', [$handler]]); - $previous_use_mode = $_SESSION['glpi_use_mode']; + $previous_use_mode = $_SESSION['glpi_use_mode']; $error_handler = $this->callPrivateConstructor( \Glpi\Application\ErrorHandler::class, @@ -154,15 +157,13 @@ public function testRegisteredErrorHandler( $_SESSION['glpi_use_mode'] = $session_mode; $error_call(); $_SESSION['glpi_use_mode'] = $previous_use_mode; - $this->integer(count($handler->getRecords()))->isEqualTo(1); - $this->boolean($handler->hasRecordThatMatches($expected_msg_pattern, $expected_log_level)); - if ( - $env === GLPI::ENV_DEVELOPMENT - || ( - $session_mode === Session::DEBUG_MODE - && ($expected_log_level->isHigherThan(Level::Info) || !in_array($env, [GLPI::ENV_STAGING, GLPI::ENV_PRODUCTION])) - ) - ) { + if ($expected_log_level !== null) { + $this->integer(count($handler->getRecords()))->isEqualTo(1); + $this->boolean($handler->hasRecordThatMatches($expected_msg_pattern, $expected_log_level)); + } else { + $this->integer(count($handler->getRecords()))->isEqualTo(0); + } + if ($expected_msg_pattern !== null && ($env === GLPI::ENV_DEVELOPMENT || $session_mode === Session::DEBUG_MODE)) { $this->output->matches($expected_msg_pattern); } else { $this->output->isEmpty(); @@ -177,6 +178,9 @@ protected function handleErrorProvider(): iterable foreach ([Session::NORMAL_MODE, Session::DEBUG_MODE] as $session_mode) { foreach ([GLPI::ENV_DEVELOPMENT, GLPI::ENV_TESTING, GLPI::ENV_STAGING, GLPI::ENV_PRODUCTION] as $env) { + $suppress_info = $env !== GLPI::ENV_DEVELOPMENT; + $suppress_notice = !in_array($env, [GLPI::ENV_DEVELOPMENT, GLPI::ENV_TESTING], true); + yield [ 'session_mode' => $session_mode, 'env' => $env, @@ -190,8 +194,8 @@ protected function handleErrorProvider(): iterable 'session_mode' => $session_mode, 'env' => $env, 'error_code' => E_NOTICE, - 'expected_log_level' => Level::Notice, - 'expected_msg_pattern' => $log_prefix . 'PHP Notice' . $log_suffix, + 'expected_log_level' => $suppress_notice ? null : Level::Notice, + 'expected_msg_pattern' => $suppress_notice ? null : $log_prefix . 'PHP Notice' . $log_suffix, 'is_fatal_error' => false, ]; @@ -226,8 +230,8 @@ protected function handleErrorProvider(): iterable 'session_mode' => $session_mode, 'env' => $env, 'error_code' => E_USER_NOTICE, - 'expected_log_level' => Level::Notice, - 'expected_msg_pattern' => $log_prefix . 'PHP User Notice' . $log_suffix, + 'expected_log_level' => $suppress_notice ? null : Level::Notice, + 'expected_msg_pattern' => $suppress_notice ? null : $log_prefix . 'PHP User Notice' . $log_suffix, 'is_fatal_error' => false, ]; @@ -235,8 +239,8 @@ protected function handleErrorProvider(): iterable 'session_mode' => $session_mode, 'env' => $env, 'error_code' => E_DEPRECATED, - 'expected_log_level' => Level::Info, - 'expected_msg_pattern' => $log_prefix . 'PHP Deprecated function' . $log_suffix, + 'expected_log_level' => $suppress_info ? null : Level::Info, + 'expected_msg_pattern' => $suppress_info ? null : $log_prefix . 'PHP Deprecated function' . $log_suffix, 'is_fatal_error' => false, ]; @@ -244,8 +248,8 @@ protected function handleErrorProvider(): iterable 'session_mode' => $session_mode, 'env' => $env, 'error_code' => E_USER_DEPRECATED, - 'expected_log_level' => Level::Info, - 'expected_msg_pattern' => $log_prefix . 'PHP User deprecated function' . $log_suffix, + 'expected_log_level' => $suppress_info ? null : Level::Info, + 'expected_msg_pattern' => $suppress_info ? null : $log_prefix . 'PHP User deprecated function' . $log_suffix, 'is_fatal_error' => false, ]; } @@ -261,20 +265,21 @@ public function testHandleError( int $session_mode, string $env, int $error_code, - Level $expected_log_level, - string $expected_msg_pattern, + ?Level $expected_log_level, + ?string $expected_msg_pattern, bool $is_fatal_error ) { $handler = new TestHandler(); $logger = $this->newMockInstance('Monolog\\Logger', null, null, ['test-logger', [$handler]]); - $previous_use_mode = $_SESSION['glpi_use_mode']; + $previous_use_mode = $_SESSION['glpi_use_mode']; $error_handler = $this->callPrivateConstructor( \Glpi\Application\ErrorHandler::class, [$logger, $env] ); $error_handler->setForwardToInternalHandler(false); + $error_handler->register(); // Assert that nothing is logged when using '@' operator $_SESSION['glpi_use_mode'] = $session_mode; @@ -305,16 +310,13 @@ public function testHandleError( $_SESSION['glpi_use_mode'] = $previous_use_mode; } - $this->integer(count($handler->getRecords()))->isEqualTo(1); - $this->boolean($handler->hasRecordThatMatches($expected_msg_pattern, $expected_log_level)); - - if ( - $env === GLPI::ENV_DEVELOPMENT - || ( - $session_mode === Session::DEBUG_MODE - && ($expected_log_level->isHigherThan(Level::Info) || !in_array($env, [GLPI::ENV_STAGING, GLPI::ENV_PRODUCTION])) - ) - ) { + if ($expected_log_level !== null) { + $this->integer(count($handler->getRecords()))->isEqualTo(1); + $this->boolean($handler->hasRecordThatMatches($expected_msg_pattern, $expected_log_level)); + } else { + $this->integer(count($handler->getRecords()))->isEqualTo(0); + } + if ($expected_msg_pattern !== null && ($env === GLPI::ENV_DEVELOPMENT || $session_mode === Session::DEBUG_MODE)) { $this->output->matches($expected_msg_pattern); } else { $this->output->isEmpty(); diff --git a/tests/functional/Glpi/Http/LegacyAssetsListener.php b/tests/functional/Glpi/Http/LegacyAssetsListener.php index 71a1a781739..56352fe78a8 100644 --- a/tests/functional/Glpi/Http/LegacyAssetsListener.php +++ b/tests/functional/Glpi/Http/LegacyAssetsListener.php @@ -594,7 +594,9 @@ public function testServeLegacyAssetsFromDeprecatedMarketplacePath(): void $response = null; $this->when( function () use ($request, &$response) { + $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations $response = $this->callPrivateMethod($this->testedInstance, 'serveLegacyAssets', $request); + \error_reporting($reporting_level); // restore previous level } )->error ->withMessage('Accessing the plugins resources from the `/marketplace/` path is deprecated. Use the `/plugins/` path instead.') diff --git a/tests/functional/Glpi/Http/LegacyRouterListener.php b/tests/functional/Glpi/Http/LegacyRouterListener.php index 649aea79ba4..9a54e960c61 100644 --- a/tests/functional/Glpi/Http/LegacyRouterListener.php +++ b/tests/functional/Glpi/Http/LegacyRouterListener.php @@ -594,7 +594,9 @@ public function testRunLegacyRouterFromDeprecatedMarketplacePath(): void $this->when( function () use ($event) { + $reporting_level = \error_reporting(E_ALL); // be sure to report deprecations $this->testedInstance->onKernelRequest($event); + \error_reporting($reporting_level); // restore previous level } )->error ->withMessage('Accessing the plugins resources from the `/marketplace/` path is deprecated. Use the `/plugins/` path instead.')