Skip to content

Conversation

matbcvo
Copy link
Contributor

@matbcvo matbcvo commented Jul 19, 2025

Q A
Bug fix? (use the a.b branch) ✔️❌
New feature/enhancement? (use the a.x branch) ✔️❌
Deprecations? ✔️❌
BC breaks? (use the c.x branch) ✔️❌
Automated tests included? ✔️❌
Related user documentation PR URL mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes #...

Description

This PR resolves several PHP 8.4 deprecations by upgrading predis/predis from v1.1.10 to v3.0.1, along with code changes required by breaking changes in Predis v3.

Below is the list of PHP 8.4 deprecations that were reported:

2 tests triggered 7 PHP deprecations:

1) /home/runner/work/mautic/mautic/vendor/predis/predis/src/Client.php:429
Predis\Client::createPipeline(): Implicitly marking parameter $options as nullable is deprecated, the explicit nullable type must be used instead

Triggered by:

* Mautic\CoreBundle\Tests\Unit\Helper\PRedisConnectionHelperTest::testCreateClientWithoutSentinel
  /home/runner/work/mautic/mautic/app/bundles/CoreBundle/Tests/Unit/Helper/PRedisConnectionHelperTest.php:71

2) /home/runner/work/mautic/mautic/vendor/predis/predis/src/Client.php:472
Predis\Client::createTransaction(): Implicitly marking parameter $options as nullable is deprecated, the explicit nullable type must be used instead

Triggered by:

* Mautic\CoreBundle\Tests\Unit\Helper\PRedisConnectionHelperTest::testCreateClientWithoutSentinel
  /home/runner/work/mautic/mautic/app/bundles/CoreBundle/Tests/Unit/Helper/PRedisConnectionHelperTest.php:71

3) /home/runner/work/mautic/mautic/vendor/predis/predis/src/Client.php:504
Predis\Client::createPubSub(): Implicitly marking parameter $options as nullable is deprecated, the explicit nullable type must be used instead

Triggered by:

* Mautic\CoreBundle\Tests\Unit\Helper\PRedisConnectionHelperTest::testCreateClientWithoutSentinel
  /home/runner/work/mautic/mautic/app/bundles/CoreBundle/Tests/Unit/Helper/PRedisConnectionHelperTest.php:71

4) /home/runner/work/mautic/mautic/vendor/predis/predis/src/Connection/Aggregate/PredisCluster.php:37
Predis\Connection\Aggregate\PredisCluster::__construct(): Implicitly marking parameter $strategy as nullable is deprecated, the explicit nullable type must be used instead

Triggered by:

* Mautic\CoreBundle\Tests\Unit\Helper\PRedisConnectionHelperTest::testCreateClientWithoutSentinel
  /home/runner/work/mautic/mautic/app/bundles/CoreBundle/Tests/Unit/Helper/PRedisConnectionHelperTest.php:71

5) /home/runner/work/mautic/mautic/vendor/predis/predis/src/Cluster/PredisStrategy.php:29
Predis\Cluster\PredisStrategy::__construct(): Implicitly marking parameter $distributor as nullable is deprecated, the explicit nullable type must be used instead

Triggered by:

* Mautic\CoreBundle\Tests\Unit\Helper\PRedisConnectionHelperTest::testCreateClientWithoutSentinel
  /home/runner/work/mautic/mautic/app/bundles/CoreBundle/Tests/Unit/Helper/PRedisConnectionHelperTest.php:71

6) /home/runner/work/mautic/mautic/vendor/predis/predis/src/Profile/RedisProfile.php:124
Predis\Profile\RedisProfile::setProcessor(): Implicitly marking parameter $processor as nullable is deprecated, the explicit nullable type must be used instead

Triggered by:

* Mautic\CoreBundle\Tests\Unit\Helper\PRedisConnectionHelperTest::testCreateClientWithoutSentinel
  /home/runner/work/mautic/mautic/app/bundles/CoreBundle/Tests/Unit/Helper/PRedisConnectionHelperTest.php:71

7) /home/runner/work/mautic/mautic/vendor/predis/predis/src/Connection/Aggregate/SentinelReplication.php:110
Predis\Connection\Aggregate\SentinelReplication::__construct(): Implicitly marking parameter $strategy as nullable is deprecated, the explicit nullable type must be used instead

Triggered by:

* Mautic\CoreBundle\Tests\Unit\Helper\PRedisConnectionHelperTest::testCreateClientWithSentinel
  /home/runner/work/mautic/mautic/app/bundles/CoreBundle/Tests/Unit/Helper/PRedisConnectionHelperTest.php:95

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Check that all automated tests pass (CI green)

Testing this PR requires knowledge of Redis.

Copy link

codecov bot commented Jul 21, 2025

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.41%. Comparing base (7fadd03) to head (37c8399).
⚠️ Report is 448 commits behind head on 7.x.

Files with missing lines Patch % Lines
app/bundles/CoreBundle/Predis/Command/Unlink.php 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                7.x   #15287      +/-   ##
============================================
- Coverage     66.41%   66.41%   -0.01%     
- Complexity    35210    35211       +1     
============================================
  Files          2319     2319              
  Lines        141913   141915       +2     
============================================
- Hits          94257    94256       -1     
- Misses        47656    47659       +3     
Files with missing lines Coverage Δ
...ndles/CoreBundle/Helper/PRedisConnectionHelper.php 87.50% <100.00%> (-5.98%) ⬇️
app/bundles/CoreBundle/Predis/Command/Unlink.php 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@matbcvo matbcvo marked this pull request as ready for review July 21, 2025 10:43
@matbcvo matbcvo requested review from fedys and escopecz July 21, 2025 10:48
@escopecz escopecz requested a review from Copilot July 21, 2025 10:52
@escopecz escopecz added deprecation Includes deprecations dependencies Pull requests that update a dependency file labels Jul 21, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR upgrades the Predis Redis client library from version 1.1.10 to 3.0.1 to resolve PHP 8.4 deprecation warnings related to implicit nullable parameter types. The upgrade includes necessary code changes to accommodate breaking changes in Predis v3's API.

Key changes include:

  • Updating composer dependency from predis/predis: ^1.1 to ^3.0
  • Adapting to new namespace structure and API changes in Predis v3
  • Modifying custom command implementation to work with the new command factory system

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

File Description
app/composer.json Updates Predis dependency version constraint to ^3.0
app/bundles/CoreBundle/Helper/PRedisConnectionHelper.php Updates imports, command registration, and connection handling for Predis v3 compatibility
app/bundles/CoreBundle/Tests/Unit/Helper/PRedisConnectionHelperTest.php Updates test to use new command factory API instead of deprecated profile methods
app/bundles/CoreBundle/Predis/Command/Unlink.php Refactors custom command to implement new Predis v3 command interface

@escopecz escopecz added this to the 7.0.0-alpha milestone Jul 21, 2025
Copy link

Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes look good. The test confirms that the Unlink::ID is present. Thank you!

@escopecz escopecz merged commit 4aa7868 into mautic:7.x Jul 21, 2025
27 checks passed
@escopecz escopecz added bug Issues or PR's relating to bugs refactoring The change does not change behavior but improves the code labels Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs dependencies Pull requests that update a dependency file deprecation Includes deprecations refactoring The change does not change behavior but improves the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants