Problem/Motivation

Hello project maintainers,

This is an automated issue to help make this module compatible with Drupal 10.

To read more about this effort by the Drupal Association, please read: The project update bot is being refreshed to support Drupal 10 readiness of contributed projects

Patches will periodically be added to this issue that remove Drupal 10 deprecated API uses. To stop further patches from being posted, change the status to anything other than Active, Needs review, Needs work or Reviewed and tested by the community. Alternatively, you can remove the "ProjectUpdateBotD10" tag from the issue to stop the bot from posting updates.

The patches will be posted by the Project Update Bot official user account. This account will not receive any issue credit contributions for itself or any company.

Proposed resolution

You have a few options for how to use this issue:

  1. Accept automated patches until this issue is closed

    If this issue is left open (status of Active, Needs review, Needs work or Reviewed and tested by the community) and the "ProjectUpdateBotD10" tag is left on this issue, new patches will be posted periodically if new deprecation fixes are needed.

    As the Drupal Rector project improves and is able to fix more deprecated API uses, the patches posted here will cover more of the deprecated API uses in the module.

    Patches and/or merge requests posted by others are ignored by the bot, and general human interactions in the issue do not stop the bot from posting updates, so feel free to use this issue to refine bot patches. The bot will still post new patches then if there is a change in the new generated patch compared to the patch that the bot posted last. Those changes are then up to humans to integrate.

  2. Leave open but stop new automated patches.

    If you want to use this issue as a starting point to remove deprecated API uses but then don't want new automated patches, remove the "ProjectUpdateBotD10" tag from the issue and use it like any other issue (the status does not matter then). If you want to receive automated patches again, add back the "ProjectUpdateBotD10" tag.

  3. Close it and don't use it

    If the maintainers of this project don't find this issue useful, they can close this issue (any status besides Active, Needs review, Needs work and Reviewed and tested by the community) and no more automated patches will be posted here.

    If the issue is reopened, then new automated patches will be posted.

    If you are using another issue(s) to work on Drupal 10 compatibility it would be very useful to other contributors to add those issues as "Related issues" when closing this issue.

Remaining tasks

Using the patches

  1. Apply the latest patch in the comments by Project Update Bot or human contributors that made it better.
  2. Thoroughly test the patch. These patches are automatically generated so they haven't been tested manually or automatically.
  3. Provide feedback about how the testing went. If you can improve the patch, post an updated patch here.

Providing feedback

If there are problems with one of the patches posted by the Project Update Bot, such as it does not correctly replace a deprecation, you can file an issue in the Drupal Rector issue queue. For other issues with the bot, for instance if the issue summary created by the bot is unclear, use the Project analysis issue queue.

Issue fork cloudflare-3296796

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Project Update Bot created an issue. See original summary.

Project Update Bot’s picture

Status: Active » Needs review
FileSize
14.75 KB

This is an automated patch generated by Drupal Rector. Please see the issue summary for more details.

It is important that any automated tests available are run with this patch and that you manually test this patch.

Drupal 10 Compatibility

According to the Upgrade Status module, even with this patch, this module is not yet compatible with Drupal 10.

Currently Drupal Rector, version 0.13.0, cannot fix all Drupal 10 compatibility problems.

This patch does not update the info.yml file for Drupal 10 compatibility.

Leaving this issue open, even after committing the current patch, will allow the Project Update Bot to post additional Drupal 10 compatibility fixes as they become available in Drupal Rector.

Debug info

Bot run #139

This patch was created using these packages:

  1. mglaman/phpstan-drupal: 1.1.24
  2. palantirnet/drupal-rector: 0.13.0

balintpekker made their first commit to this issue’s fork.

balintpekker’s picture

Issue summary: View changes

There is a proposal to switch CloudFlare library, which either needs to be done or the existing one needs to be updated to use Guzzle 7. The d8-contrib-modules/cloudflarephpsdk is using guzzlehttp/guzzle ~6.1 hence the module won't be D10 compatible until it is resolved. The library has a pull request to allow Guzzle 7, the only thing needed is to merge it.

Project Update Bot’s picture

This is an automated patch generated by Drupal Rector. Please see the issue summary for more details.

It is important that any automated tests available are run with this patch and that you manually test this patch.

Drupal 10 Compatibility

According to the Upgrade Status module, even with this patch, this module is not yet compatible with Drupal 10.

Currently Drupal Rector, version 0.13.1, cannot fix all Drupal 10 compatibility problems.

This patch does not update the info.yml file for Drupal 10 compatibility.

Leaving this issue open, even after committing the current patch, will allow the Project Update Bot to post additional Drupal 10 compatibility fixes as they become available in Drupal Rector.

Debug info

Bot run #145

This patch was created using these packages:

  1. mglaman/phpstan-drupal: 1.1.25
  2. palantirnet/drupal-rector: 0.13.1
balintpekker’s picture

The rector patch posted in #6 is already part of MR !7

nkoporec’s picture

Status: Needs review » Needs work

Reviewed the MR, left some questions, so setting this a 'Needs work' for now.

balintpekker’s picture

Status: Needs work » Needs review
nkoporec’s picture

Status: Needs review » Reviewed & tested by the community

The changes looks good, marking as RTBC.

Project Update Bot’s picture

Issue summary: View changes
jcnventura’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

Except that it doesn't really work since trying to install this on Drupal 10 will result in:

  Problem 1
    - d8-contrib-modules/cloudflarephpsdk[1.0.0-alpha1, ..., 1.0.0-alpha5] require guzzlehttp/guzzle ~6.1 -> found guzzlehttp/guzzle[6.1.0, ..., 6.5.8] but the package is fixed to 7.5.0 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
    - drupal/cloudflare[1.0.0-beta1, ..., 1.0.0-beta2] require d8-contrib-modules/cloudflarephpsdk ^1 -> satisfiable by d8-contrib-modules/cloudflarephpsdk[1.0.0-alpha1, ..., 1.0.0-alpha5].
    - Root composer.json requires drupal/cloudflare ^1.0@beta -> satisfiable by drupal/cloudflare[1.0.0-beta1, 1.0.0-beta2].

Looking into the issue queue of contrib-modules/cloudflarephpsdk, I see the following issue: Deprecate this library in favour of the official one

As such, Drupal 10 support is blocked on progress of #2915341: Proposal to switch Cloudflare library. Once the official library is being used, we can then think about adding Drupal 10 support.

balintpekker’s picture

@jcnventura is right, please see comment #5, and PR #38 for cloudfarephpsdk: https://github.com/d8-contrib-modules/cloudflarephpsdk/pull/38

mglaman’s picture

Status: Needs work » Reviewed & tested by the community
jcnventura’s picture

As per https://www.drupal.org/node/3070687#test, test modules should be in Testing package, and skip core_version_requirement.

VladimirAus’s picture

Thanks everyone! Hiding files so the focus is on MR.
Would be great to create 2.0.x branch and release for D10 support.
Happy to help maintain the module if required.

You can test Drupal 10 version by running

composer require drupal/cloudflare:dev-3296796-automated-drupal-10

and adding

  "repositories": [
    {
      "type": "vcs",
      "url": "https://git.drupalcode.org/issue/cloudflare-3296796.git"
    },
...

before main drupal repository.

jcnventura’s picture

@VladimirAus Use https://github.com/mglaman/composer-drupal-lenient + composer patches instead of adding custom repos when testing D10 support. It's a lot simpler :)

jcnventura’s picture

Status: Reviewed & tested by the community » Needs review

Taking into account #3280603: Fix tests in 8.x-1.x, I've rebased the MR, and undid all the changes from base 8.x-1.x that don't seem to be necessary for D10 support. If the tests pass, this should be a much easier change. I need to investigate when ResponseEvent and RequestEvent were made available in Symfony, as it may be that the Drupal 8 compatibility needs to be dropped.

VladimirAus’s picture

Status: Needs review » Reviewed & tested by the community

Looking good.
Patch applies and doesn't break D10.
Cheers 🍻

jcnventura’s picture

Status: Reviewed & tested by the community » Needs work

Well, the tests failed in D10. The fix is trivial, however. Let me just fix that and see if we need to supplement #3280603: Fix tests in 8.x-1.x further..

jcnventura’s picture

Status: Needs work » Needs review

OK. All tests pass in Drupal 9.5 and Drupal 10.1. I dropped support for Drupal 8, since the new ResponseEvent and RequestEvent classes that are no longer available in Symfony 6 (used by Drupal 10) were first marked deprecated in Symfony 4.3 (which Drupal 9 uses).

Janec made their first commit to this issue’s fork.

RoSk0’s picture

Status: Needs review » Needs work

This needs rebase now when #3280262: Support more use cases is committed.

I'm curious is we would be able to support 9 and 10 on the same branch with the changes from symfony/http-kernel , like https://www.drupal.org/node/3236639 .

I will try to do it myself as it wouldn't be trivial with the IP restore functionality moved into middleware.

RoSk0’s picture

Status: Needs work » Needs review

Rebased and bypassHost property declaration on the middleware class (was breaking tests on PHP 8.2).

Lets see if tests are green now.

mandclu made their first commit to this issue’s fork.

mandclu’s picture

The bigger issue with the change from MASTER_REQUEST to MAIN_REQUEST in Symfony's HttpKernelInterface is that now it's impossible to have a CloudFlareMiddleware that doesn't throw a fatal error in either Drupal 9.5 or 10, at least as long as it continues to implement HttpKernelInterface.

For now I've updated it to work with Drupal 10, since that is ultimately the aim of the work here. I also added an ability for the SettingsForm to gracefully handle a ResponseException from the Cloudflare SDK.

It's down to one error on my local, I'll have to do some more investigation to figure out why the testbot isn't showing any improvement.

jcnventura’s picture

Yes, unfortunately now that #3280262: Support more use cases has been committed and made available in a released version it seems that there needs to be a version for Drupal 10 and another for Drupal 9.

Please create a 2.x branch, and move this issue over to that branch.

mandclu’s picture

As of the last two commits, the tests pass for Drupal 10, at least in PHP 8.1. The most recent commit resolves some deprecation errors related to dynamic properties in PHP 8.2, but there are still a number thrown related to ctools, so I've proposed changes at #3346063: Dynamic properties are deprecated PHP 8.2, explicitly declare WizardFormController::$wizardFactory that will hopefully resolve the remainder.

As for moving this issue to a new major version, there's an explanation in this article about why adding major version support in a new major version of a module creates headaches for site maintainers. Does CloudFlareMiddleware need to implement HttpKernelInterface? Could we remove that from the 8.x-1.x branch and tag a release compatible with both 9.x and 10, and then later tag a 2.0 release that adds the HttpKernelInterface implementation back in, and only support Drupal 10+. Or, we could tag a new 2.0.x release without the implementation, and then later make a 2.1.x branch that adds it back in.

mandclu’s picture

Removing the HttpKernelInterface implementation didn't work so it looks like going to a 2.0.x branch is the only path forward, even though it will be inconvenient for site owners looking to upgrade to Drupal 10. If we're ready to move forward with that approach I can create the new branch and merge in these changes once they've been tested by someone else.

Testing locally with these changes and the patched version of ctools, the PHP 8.2 tests pass as well.

VladimirAus’s picture

Status: Needs review » Reviewed & tested by the community

Happy with it. Thanks 🍻

jcnventura’s picture

I'd suggest a 2.x branch instead of a 2.0.x branch, as you can easily create 2.1.0, 2.2.0 versions out of a 2.x branch, but you'll need to keep doing many 2.y.x branches if you start with the 2.0.x branch... But you guys are the maintainers, so do whatever you think is best.

  • mandclu committed e4a83fbf on 2.0.x
    Issue #3296796 by jcnventura, balintpekker, mandclu, Project Update Bot...

  • RoSk0 committed 01978819 on 2.0.x
    #3296796: Declare bypassHost property
    

mandclu’s picture

Version: 8.x-1.x-dev » 2.0.x-dev
Status: Reviewed & tested by the community » Fixed

I merged this into a new 2.0.x branch and rolled a 2.0.0-alpha1 release. My intent is to soon roll an alpha2 release that will only support Drupal 10, but I thought for the sake of upgrading that it was important to have at least one release that supported both major versions, even if it will cause fatal errors in Drupal 9.

Open to opinions on how quickly to release the alpha2.

RoSk0’s picture

Thanks for pushing this Martin.

Can you please elaborate why you choose 2.0.x branch , rather than suggested in #31 2.x? Major version branches make more sense in semver context , easier to maintain.

I would really prefer to see 2.x branch with corresponding 2.x-dev release for the latest changes.

mandclu’s picture

In my experience, feature version branches, e.g. 2.0.x are what the community intends. For example, when you create a new project, the suggested git code creates a 1.0.x branch, not 1.x. Further, in my experience a feature version dev branch displays coupled with the corresponding tagged release, as the 2.0.x-dev and 2.0.0-alpha1 display together on this project page currently.

On a more practical note, feature version branches allow for the development of new features in a separate branch while still being able to push maintenance fixes to the current stable branch. Once we get a stable 2.0.0 release, we can work on new features in a 2.1.x branch, while still being able to tag new bugfix releases e.g. 2.0.1.

RoSk0’s picture

Thanks. All good, as long as it's a conscious choice.

RoSk0’s picture

Status: Fixed » Needs work

It looks like we are not done here - 2.0.x is still marked compatible with Drupal core 9, which is not true.

Also test are failing on PHP 8.2, but we can address that in another issue.

mandclu’s picture

It was mentioned above that because of deeper changes in the versions of Symfony used by D9 and D10 (and this module's reliance on classes whose definitions have changed) it is impossible to have a release that supports both major versions, at least given other changes that have been merged in since this issue was started.

In the release notes for the 2.0.0-alpha1 release, it clearly indicates that the release should only be used by Drupal 9 sites that are preparing to upgrade to Drupal 10. I myself followed this process this weekend, and it worked seamlessly. Perhaps additional documentation on that front is warranted.

My plan is to tag a 2.0.0-alpha2 release that will only be compatible with Drupal 10, but having at least one release that composer thinks is compatible with both makes it easier to achieve the major version upgrade for Drupal.

IT-Cru’s picture

@mandclu: Would it than not be better to use 2.0.x for D9 / D10 and 2.1.x for D10 only. So it will be possible to provide fixes also for D9 compatible 2.0.x release for a while instead of blocking all development here with a 2.0.0-alpha2 which only supports D10.

IT-Cru’s picture

I've tried out 2.0.0-alpha1 with D9 which produce fatal errors like mentioned in info of this release. I think a lot of people could run into this issue, when they use upgrade_status module during D10 preparation and get a non-working cloudflare release installed in their D9 websites.

Would it be possible to avoid this with a new 2.0.0-alpha2 release which only supports D10 and mark 2.0.0-alpha1 as unsupported?

@mandclu: So I think you could ignore my previous comment #42 ;)

kuhikar’s picture

Hello Team,

Added Patch : This is compatible for the Drupal 9 (9.5.7) and Drupal 10 (10.0.8-dev).

Created Patch on https://git.drupalcode.org/project/cloudflare/-/tree/2.0.x-dev

D10 Readiness

Patch Applied on Drupal 9 and Drupal 10:

1. Drupal 9:

Checking patch cloudflare.info.yml...
Checking patch cloudflare.install...
Checking patch modules/cloudflarepurger/cloudflarepurger.info.yml...
Checking patch modules/cloudflarepurger/src/EventSubscriber/CloudFlareCacheTagHeaderGenerator.php...
Checking patch modules/cloudflarepurger/src/Plugin/Purge/Purger/CloudFlarePurger.php...
Checking patch modules/cloudflarepurger/tests/src/Unit/DiagnosticCheckTestBase.php...
Checking patch src/CloudFlareMiddleware.php...
Checking patch src/Form/ZoneSelectionForm.php...
Checking patch src/State.php...
Checking patch src/Zone.php...
Checking patch tests/modules/cloudflare_form_tester/cloudflare_form_tester.info.yml...
Checking patch tests/src/Functional/CloudFlareAdminSettingsFormTest.php...
Checking patch tests/src/Functional/CloudFlareAdminSettingsInvalidFormTest.php...
Checking patch tests/src/Functional/ComposerDependencyTest.php...
Checking patch tests/src/Unit/ClientIpRestoreTest.php...
Applied patch cloudflare.info.yml cleanly.
Applied patch cloudflare.install cleanly.
Applied patch modules/cloudflarepurger/cloudflarepurger.info.yml cleanly.
Applied patch modules/cloudflarepurger/src/EventSubscriber/CloudFlareCacheTagHeaderGenerator.php cleanly.
Applied patch modules/cloudflarepurger/src/Plugin/Purge/Purger/CloudFlarePurger.php cleanly.
Applied patch modules/cloudflarepurger/tests/src/Unit/DiagnosticCheckTestBase.php cleanly.
Applied patch src/CloudFlareMiddleware.php cleanly.
Applied patch src/Form/ZoneSelectionForm.php cleanly.
Applied patch src/State.php cleanly.
Applied patch src/Zone.php cleanly.
Applied patch tests/modules/cloudflare_form_tester/cloudflare_form_tester.info.yml cleanly.
Applied patch tests/src/Functional/CloudFlareAdminSettingsFormTest.php cleanly.
Applied patch tests/src/Functional/CloudFlareAdminSettingsInvalidFormTest.php cleanly.
Applied patch tests/src/Functional/ComposerDependencyTest.php cleanly.
Applied patch tests/src/Unit/ClientIpRestoreTest.php cleanly.

2. Drupal 10:

cloudflare.patch:109: trailing whitespace.
  
cloudflare.patch:152: trailing whitespace.
   * 
cloudflare.patch:166: trailing whitespace.
   * 
cloudflare.patch:173: trailing whitespace.
   * 
cloudflare.patch:187: trailing whitespace.
   * 
Checking patch cloudflare.info.yml...
Checking patch cloudflare.install...
Checking patch modules/cloudflarepurger/src/EventSubscriber/CloudFlareCacheTagHeaderGenerator.php...
Checking patch modules/cloudflarepurger/src/Plugin/Purge/Purger/CloudFlarePurger.php...
Checking patch modules/cloudflarepurger/tests/src/Unit/DiagnosticCheckTestBase.php...
Checking patch src/CloudFlareMiddleware.php...
Checking patch src/Form/ZoneSelectionForm.php...
Checking patch src/State.php...
Checking patch src/Zone.php...
Checking patch tests/src/Functional/CloudFlareAdminSettingsFormTest.php...
Checking patch tests/src/Functional/CloudFlareAdminSettingsInvalidFormTest.php...
Checking patch tests/src/Functional/ComposerDependencyTest.php...
Checking patch tests/src/Unit/ClientIpRestoreTest.php...
Applied patch cloudflare.info.yml cleanly.
Applied patch cloudflare.install cleanly.
Applied patch modules/cloudflarepurger/src/EventSubscriber/CloudFlareCacheTagHeaderGenerator.php cleanly.
Applied patch modules/cloudflarepurger/src/Plugin/Purge/Purger/CloudFlarePurger.php cleanly.
Applied patch modules/cloudflarepurger/tests/src/Unit/DiagnosticCheckTestBase.php cleanly.
Applied patch src/CloudFlareMiddleware.php cleanly.
Applied patch src/Form/ZoneSelectionForm.php cleanly.
Applied patch src/State.php cleanly.
Applied patch src/Zone.php cleanly.
Applied patch tests/src/Functional/CloudFlareAdminSettingsFormTest.php cleanly.
Applied patch tests/src/Functional/CloudFlareAdminSettingsInvalidFormTest.php cleanly.
Applied patch tests/src/Functional/ComposerDependencyTest.php cleanly.
Applied patch tests/src/Unit/ClientIpRestoreTest.php cleanly.
warning: squelched 3 whitespace errors
warning: 8 lines add whitespace errors

Please include this patch in the alpha-2 release

kuhikar’s picture

Status: Needs work » Reviewed & tested by the community
kuhikar’s picture

Status: Reviewed & tested by the community » Needs review
socketwench’s picture

While not a solution to this issue, this patch can help site maintainers who fall upon this fatal error with the 2.0.0-alpha1 version while on D9 as a stopgap.

https://www.drupal.org/files/issues/2023-07-13/cloudflare_2.0.0_D9_compa...

kleinmp’s picture

This patch is allowing a site I'm working on to run in both Drupal 9.5.x and 10.0.x with version 2.0 of this module. It probably does not work for Drupal 9.4 and less.

I updated the handle method to match how core is using it:

https://git.drupalcode.org/project/drupal/-/blob/10.0.11/core/modules/pa...

[Edit: Nevermind, patch #47 is working for me for both D9 and D10.]

VladimirAus’s picture

Status: Needs review » Fixed

@socketwench do you want to raise separate ticket for it?

I'm marking this issue as fixed as it was committed 6 month ago.

You can test it on D9 and D10 by running

composer require 'drupal/cloudflare:2.0.x-dev@dev'

Maintainers, do you help with releases? Happy to help.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.