[ECP-9653] Fix for Making Adyen Payments compatible with Magento 2.4.8 version and PHP 8.1+

[parekhreena]

This PR fixes compatibility issues with Magento 2.4.8 as discussed in #2914.

Fixes

• [ECP-9653] Incompatibility between Adyen 9* and Magento 2.4.8-beta* #2914.
• php bin/magento setup:upgrade command successfully runs on magento 2.4.8 version using PHP 8.1+ versions

The changes in this pull request primarily focus on updating type hints in various PHP files to improve compatibility with PHP 8.1+ and Magento 2.4.8. Additionally, the logging classes have been refactored to utilize the standard Monolog Level enum, which is a good practice. The changes appear to be well-implemented and directly address the stated objective of the pull request.

Summary of Findings

• PHP 8.1+ Type Hinting: The pull request correctly introduces nullable type hints (?) and union types (int|string|null) to function parameters and properties where appropriate, aligning the code with stricter type checking requirements in PHP 8.1+.
  -Monolog Level Enum Usage: The custom log levels in AdyenLogger have been replaced with the standard Monolog\Level enum, and the corresponding handlers have been updated. This improves consistency and adherence to the Monolog library's practices.
• Severity Assessment: Based on the review criteria and the requested severity levels (medium, high, critical), no issues falling within these categories were identified in the code changes.

[gemini-code-assist]

Hello @parekhreena, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Gemini here, providing a summary of this pull request. The primary goal of this PR, as indicated by the title, is to ensure compatibility of the Adyen Payments module with Magento 2.4.8 and PHP 8.1+. The changes primarily involve updating the codebase to leverage modern PHP features, specifically focusing on improved type hinting and updating the logging implementation to align with newer standards.

Highlights

• PHP 8.1+ Type Hinting: Extensive use of nullable type hints (?) and union type hints (int|string|null) has been added to method and constructor parameters across various files (Helpers, Models, Controllers, Tests). This improves code clarity, enables better static analysis, and ensures compatibility with stricter type checking in PHP 8.1+.
• Monolog Logging Update: The custom log levels (ADYEN_DEBUG, ADYEN_NOTIFICATION, ADYEN_RESULT) within the AdyenLogger have been removed and replaced with standard Monolog\Level enums. The corresponding logger handlers have also been updated to use these standard levels.
• Constructor Parameter Nullability: Nullable type hints have been added to optional constructor parameters in several Model and Backend Config classes, particularly for dependencies like resource models, collections, and factories, aligning with common Magento dependency injection patterns.

Changelog

Click here to see the changelog

• Controller/Return/Index.php
  • Added nullable type hint ?string to the $incrementId parameter in the getOrder method (line 176).
• Helper/Config.php
  • Added union type hint int|string|null to the $storeId parameter in numerous methods (getApiKey, getClientKey, getIsPaymentMethodsActive, getMerchantAccount, getMotoMerchantAccounts, isMotoPaymentMethodEnabled, getMotoMerchantAccountProperties, getNotificationsUsername, getNotificationsPassword, getWebhookUrl, getWebhookId, getNotificationsCanCancel, getNotificationsHmacKey, getNotificationsIpCheck, isDemoMode, isAlternativePaymentMethodsEnabled, getChargedCurrency, getHasHolderName, getHouseNumberStreetLine, getHolderNameRequired, getAdyenPosCloudConfigData, useQueueProcessor, getAdyenAbstractConfigData, getLiveEndpointPrefix, getAdyenAbstractConfigDataFlag, getAdyenCcConfigData, getAdyenHppConfigData, getAdyenHppVaultConfigDataFlag, isHppVaultEnabled, getAdyenOneclickConfigData, getAdyenOneclickConfigDataFlag, getAdyenBoletoConfigData, getCheckoutFrontendRegion, getRatePayId, getAllowMultistoreTokens, getThreeDSFlow, getIsCvcRequiredForRecurringCardPayments, getConfigData).
  • Added string type hint to the $mode parameter in getApiKey and getClientKey (lines 85, 97).
  • Added nullable type hint ?int to the $storeId parameter in getNotificationsIpCheck (line 269).
  • Added union type hint int|string|null to the $storeId parameter in getConfigData (line 642).
• Helper/ConnectedTerminals.php
  • Added nullable type hint ?int to the $storeId parameter in the getConnectedTerminals method (line 34).
• Helper/Data.php
  • Added nullable type hint ?Client to the $client parameter in the createAdyenCheckoutService method (line 1445).
• Logger/AdyenLogger.php
  • Removed custom log level constants ADYEN_DEBUG, ADYEN_NOTIFICATION, and ADYEN_RESULT (lines 25-27).
  • Removed custom levels from the static $levels array (lines 36, 38-39).
  • Updated addAdyenNotification to use Level::Notice instead of static::ADYEN_NOTIFICATION (line 68).
  • Updated addAdyenDebug to use Level::Debug instead of static::ADYEN_DEBUG (line 75).
  • Updated addAdyenResult to use Level::Debug instead of static::ADYEN_RESULT (line 86).
• Logger/Handler/AdyenBase.php
  • Imported Monolog\LogRecord (line 17).
  • Updated the isHandling method signature to use LogRecord type hint for the $record parameter (line 47).
• Logger/Handler/AdyenDebug.php
  • Imported Monolog\Level (line 16).
  • Updated $loggerType property to use Level::Debug (line 28).
  • Updated $level property to use Level::Debug and added Level type hint (line 30).
• Logger/Handler/AdyenError.php
  • Imported Monolog\Level (line 16).
  • Updated $level property to use Level::Error and added Level type hint (line 33).
• Logger/Handler/AdyenInfo.php
  • Imported Monolog\Level (line 16).
  • Updated $level property to use Level::Info and added Level type hint (line 30).
• Logger/Handler/AdyenNotification.php
  • Imported Monolog\Level (line 16).
  • Updated $loggerType property to use Level::Notice (line 28).
  • Updated $level property to use Level::Notice and added Level type hint (line 30).
• Logger/Handler/AdyenResult.php
  • Imported Monolog\Level (line 16).
  • Updated $loggerType property to use Level::Debug (line 28).
  • Updated $level property to use Level::Debug and added Level type hint (line 30).
• Logger/Handler/AdyenWarning.php
  • Imported Monolog\Level (line 15).
  • Updated $level property to use Level::Warning and added Level type hint (line 29).
• Model/Config/Backend/AutoConfiguration.php
  • Added nullable type hints ?AbstractResource and ?AbstractDb to the $resource and $resourceCollection constructor parameters (lines 64-65).
• Model/Config/Backend/DonationAmounts.php
  • Added nullable type hints ?AbstractResource and ?AbstractDb to the $resource and $resourceCollection constructor parameters (lines 47-48).
• Model/Config/Backend/Installments.php
  • Added nullable type hints ?MagentoFrameworkModelResourceModelAbstractResource and ?MagentoFrameworkDataCollectionAbstractDb to the $resource and $resourceCollection constructor parameters (lines 43-44).
• Model/Config/Backend/InstallmentsPosCloud.php
  • Added nullable type hints ?MagentoFrameworkModelResourceModelAbstractResource and ?MagentoFrameworkDataCollectionAbstractDb to the $resource and $resourceCollection constructor parameters (lines 44-45).
• Model/Config/Backend/Moto.php
  • Added nullable type hints ?MagentoFrameworkModelResourceModelAbstractResource and ?MagentoFrameworkDataCollectionAbstractDb to the $resource and $resourceCollection constructor parameters (lines 57-58).
• Model/Config/Backend/OrderStatus.php
  • Added nullable type hints ?MagentoFrameworkModelResourceModelAbstractResource and ?MagentoFrameworkDataCollectionAbstractDb to the $resource and $resourceCollection constructor parameters (lines 46-47).
  • Reordered constructor parameters and PHPDoc block to match the new nullable type hints.
• Model/Config/Backend/PaymentMethodsStatus.php
  • Added nullable type hints ?AbstractResource and ?AbstractDb to the $resource and $resourceCollection constructor parameters (lines 33-34).
• Model/Config/Backend/ProcessedWebhookRemoval.php
  • Added nullable type hints ?AbstractResource and ?AbstractDb to the $resource and $resourceCollection constructor parameters (lines 35-36).
• Model/Config/Backend/Tokenization.php
  • Added nullable type hints ?AbstractResource and ?AbstractDb to the $resource and $resourceCollection constructor parameters (lines 50-51).
• Model/Config/Backend/WebhookCredentials.php
  • Added nullable type hints ?AbstractResource and ?AbstractDb to the $resource and $resourceCollection constructor parameters (lines 64-65).
• Model/Creditmemo.php
  • Added nullable type hints ?AbstractResource and ?AbstractDb to the $resource and $resourceCollection constructor parameters (lines 28-29).
• Model/Invoice.php
  • Added nullable type hints ?AbstractResource and ?AbstractDb to the $resource and $resourceCollection constructor parameters (lines 28-29).
• Model/Method/PaymentMethodVault.php
  • Added nullable type hint ?Json to the $jsonSerializer constructor parameter (line 40).
• Model/Notification.php
  • Added nullable type hints ?AbstractResource and ?AbstractDb to the $resource and $resourceCollection constructor parameters (lines 57-58).
• Model/Order/Payment.php
  • Added nullable type hints ?AbstractResource and ?AbstractDb to the $resource and $resourceCollection constructor parameters (lines 37-38).
• Model/Sales/OrderRepository.php
  • Added nullable type hints to several constructor parameters: ?CollectionProcessorInterface, ?OrderExtensionFactory, ?OrderTaxManagementInterface, ?PaymentAdditionalInfoInterfaceFactory, ?JsonSerializer, and ?JoinProcessorInterface (lines 43-48).
• Model/TransportBuilder.php
  • Added nullable type hints to several constructor parameters: ?TransportInterfaceFactory, ?EmailMessageInterfaceFactory, ?MimeMessageInterfaceFactory, ?MimePartInterfaceFactory, and ?AddressConverter (lines 150-154).
• Test/Unit/Gateway/Http/Client/TransactionCaptureTest.php
  • Added nullable type hints ?array and ?Exception to the $response and $exception parameters in the configureAdyenMocks method (line 56).
• Test/Unit/Helper/ManagementHelperTest.php
  • Added nullable type hints to several parameters in the createManagementHelper method: ?StoreManager, ?EncryptorInterface, ?Data, ?Config, ?AdyenLogger, and ?ManagerInterface (lines 567-572).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

Types now guide the way,
Nulls and unions hold their sway,
PHP, clear and bright.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The changes in this pull request primarily focus on updating type hints in various PHP files to improve compatibility with PHP 8.1+ and Magento 2.4.8. Additionally, the logging classes have been refactored to utilize the standard Monolog Level enum, which is a good practice. The changes appear to be well-implemented and directly address the stated objective of the pull request.

Summary of Findings

• PHP 8.1+ Type Hinting: The pull request correctly introduces nullable type hints (?) and union types (int|string|null) to function parameters and properties where appropriate, aligning the code with stricter type checking requirements in PHP 8.1+.
• Monolog Level Enum Usage: The custom log levels in AdyenLogger have been replaced with the standard Monolog\Level enum, and the corresponding handlers have been updated. This improves consistency and adherence to the Monolog library's practices.
• Severity Assessment: Based on the review criteria and the requested severity levels (medium, high, critical), no issues falling within these categories were identified in the code changes.

Merge Readiness

The code changes are focused on compatibility and minor refactoring, and no critical or high-severity issues were found during the review. The changes appear safe and ready to be merged. As a code reviewer, I am unable to directly approve the pull request, but based on this review, it seems ready for approval by another maintainer.

[candemiralp]

Hello @parekhreena,

Thank you for your contribution and we really appreciate your effort to make the plugin compatible with Magento 2.4.8.

However, the changes in the logger are backward incompatible. The incompatibility arises from the input argument type change in Monolog library here.

Due to this fact, this PR can not be merged to main branch, otherwise the plugin can not be installed on the versions prior to Magento 2.4.8. Besides that, it is against semantic versioning.

I am closing this PR as there is no further action will be taken.

Best Regards,
Can

[parekhreena]

Hi @candemiralp Based on your response, it seems there's a misunderstanding of how PRs are intended to function on GitHub. The purpose of PRs on Github is for them to be reviewed, propose any new changes rather than have them merged as-is.

If 1 file needed change, you can comment on proposing changes to that file instead of closing the whole PR with 30 other files which were correct.

I hope you reconsider your approach to an open source extension and it's contributions.

This is Especially needed on your part, as your current approach has kept your payment method upgrade incompatible since months for hundreds of merchants, thereby leaving them vulnerable to important security update.

[candemiralp]

Hello @parekhreena,

Thank you for your response and I really understand your frustration as it's currently not possible to upgrade to the latest version of Magento with our payment plugin. I ensure you that our internal team is working hard to enable our merchants again to use the latest version of the platform.

Asides from this note, we encourage our contributors to create issues and propose pull-requests on our repository. We already use Github as a platform to discuss technical approaches via comments and improve the proposed solutions by collaborating closely.

We have reviewed your PR already. However, there are some fundamental issues on your PR which can not be solved by discussing line-by-line. One of them is related to Monolog as I mentioned above and another one is related to mixed type declarations. Earlier versions of Magento (~2.4.4) does not support mixed argument type declarations (even though PHP supports it). It ends-up with a failure during DI compilation. After extracting these two changes, we observed some changes to make input types nullable, which is not bringing a real value after refactoring 30 files.

Based on the maintainability of the software that we offer to our merchants, we consider the supported versions of the platform, libraries and external dependencies at the same time. This approach ensures us to deliver what we have already committed and keep the quality always on top of everything while keeping the long-term support cycle in our mind always. Due to these facts, we can't introduce any breaking changes to a stable version even if they are for supporting the latest version of Magento.

I accept and appreciate your feedback about the timeline of the delivery of the next major version and have already posted it internally. We will keep this in mind while preparing the next year's roadmap.

Best Regards,
Can

[pbrouwers]

@parekhreena I noticed Model/Sales/OrderRepository.php also needed a fix, could you merge
parekhreena#2 into yours?

[parekhreena]

hi @pbrouwers Thank you for contributing. I have now merged your PR in to my branch.