Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CakePHP Upgrade to 4.x #3123

Draft
wants to merge 23 commits into
base: dev
Choose a base branch
from

Conversation

cblanken
Copy link
Contributor

@cblanken cblanken commented Apr 19, 2024

This isn't anywhere near complete, but I wanted to get the ball rolling on issue #3067
to upgrade to CakePHP 4.x since 3.x is EOL.

Progress

I've been following the CakePHP 4.0 Upgrade Guide. It has a few steps:

  • Update to the latest 3.x release
    • CakePHP 3.9 and 3.10 are API compatible with 3.8 so I just updated the dependencies, though someone with more experience on the project may want to take a look at the changelogs for 3.9 and 3.10 just to be sure they aren't going to introduce any subtle bugs. Nothing has come up in my testing thus far though.

Edit: I've tried to match these to individual commits mostly, so it should be apparent where each deprecation is resolved, but this is a high-level overview of all deprecations that need attention.

Abandoned packages

There are several abandoned packages still in use even after meeting the above dependency targets. It might be better to resolve these in another PR, but I'll document them here just in case.

  • Package flow/jsonpath is abandoned, you should avoid using it. Use softcreatr/jsonpath instead.
  • Package patchwork/jsqueeze is abandoned, you should avoid using it. No replacement was suggested.
  • Package laminas/laminas-zendframework-bridge is abandoned, you should avoid using it. No replacement was suggested.
  • Package phpunit/php-token-stream is abandoned, you should avoid using it. No replacement was suggested.

Remaining Deprecation Warnings

The deprecations I listed seem to be the most prolific, but as far as I can tell I got all instances of each. There are few more though.

[Deprecated (16384)](javascript:void(0);): Router::parseNamedParams() is deprecated. 2.x backwards compatible named parameter support will be removed in 4.0 - /home/vagrant/Tatoeba/src/Controller/AppController.php, line: 136
 You can disable deprecation warnings by setting `Error.errorLevel` to `E_ALL & ~E_USER_DEPRECATED` in your config/app.php. [CORE/src/Core/functions.php, line 311]
[Deprecated (16384)](javascript:void(0);): ServerRequest::query() is deprecated. Use getQuery() or the PSR-7 getQueryParams() and withQueryParams() methods instead. - /home/vagrant/Tatoeba/src/Controller/SentencesController.php, line: 509
 You can disable deprecation warnings by setting `Error.errorLevel` to `E_ALL & ~E_USER_DEPRECATED` in your config/app.php. [CORE/src/Core/functions.php, line 311]
[Warning (512)](javascript:void(0);): Unable to emit headers. Headers sent in file=/home/vagrant/Tatoeba/vendor/cakephp/cakephp/src/Error/Debugger.php line=855 [CORE/src/Http/ResponseEmitter.php, line 53]
[Warning (2)](javascript:void(0);): Cannot modify header information - headers already sent by (output started at /home/vagrant/Tatoeba/vendor/cakephp/cakephp/src/Error/Debugger.php:855) [CORE/src/Http/ResponseEmitter.php, line 154]
[Warning (2)](javascript:void(0);): Cannot modify header information - headers already sent by (output started at /home/vagrant/Tatoeba/vendor/cakephp/cakephp/src/Error/Debugger.php:855) [CORE/src/Http/ResponseEmitter.php, line 183]
[Warning (2)](javascript:void(0);): Cannot modify header information - headers already sent by (output started at /home/vagrant/Tatoeba/vendor/cakephp/cakephp/src/Error/Debugger.php:855) [CORE/src/Http/ResponseEmitter.php, line 269]

The first one listed here in particular may be a bit troublesome. I'm not familiar with all the routing done for the application, but I assume named parameters are still in use for some routes, thus the invokation of parseNamedParams(). Unfortunately, it seems 4.0 and onward won't support them, so it may be necessary to write some custom middleware to handle whatever requests are still using named parameters.

This is still a work in progress, but I'll see what I can do about the remaining deprecation warnings to get the application ready to use the 4.0 upgrade tool.

Setting `$config` is deprecated. Configurations files should instead return an array.
See https://book.cakephp.org/3/en/appendices/3-0-migration-guide.html#configure
Access to Helper::$request as a property is deprecated as of CakePHP 3.7
The getter `Helper->getRequest()` should be used instead.
See https://book.cakephp.org/3/en/appendices/3-7-migration-guide.html#deprecations

Following are some other deprecation warnings addressed while fixing the above.
- Request::param() is deprecated as of 3.4. Use Request::getParam() instead. See https://book.cakephp.org/3/en/appendices/3-4-migration-guide.html#request-response-deprecations
- ServerRequest::here() to be removed in 4.0.0. Use getRequestTarget() instead.

Note:
The SecurityComponent is deprecated and I believe SecurityHelper will need to be refactored
or removed entirely per the 4.0 Migration Guide (https://book.cakephp.org/4/en/appendices/4-0-migration-guide.html#component)
before migrating to CakePHP 5.x
See this StackOverflow post for more details: https://stackoverflow.com/a/61630537
This addresses the following CakePHP 3.x deprecation notice
> Accessing `params` as a property will be removed in 4.0.0. Use request->getParam() instead

See https://book.cakephp.org/3/en/appendices/3-4-migration-guide.html#request-response-deprecations for details
This commit addresses the following Form and FormHelper related 3.x deprecation warnings
- FormHelper::input() is deprecated. Use FormHelper::control() instead. See https://book.cakephp.org/3/en/appendices/3-4-migration-guide.html#other-deprecations.
- FormHelpter::create() should take `null` instead of a `string` or `bool` as the context when creating contextless forms. See https://book.cakephp.org/3/en/appendices/3-9-migration-guide.html#deprecations.
Fix the following deprecation warning
> Cake\Controller\Component\SecurityComponent::config() is deprecated. Use setConfig()/getConfig() instead.
I couldn't find the exact version this deprecation happens, but the SecurityComponent page
here (https://book.cakephp.org/3/en/controllers/components/security.html) implies `SecurityComponent::config()`
is deprecated after 3.4
Fix the following deprecation warning.
> Using Query::repository() as getter is deprecated. Use getRepository() instead
See https://book.cakephp.org/3/en/appendices/3-6-migration-guide.html#deprecations for details
Fix the following decprecation warning(s)
> ServerRequest::query() is deprecated. Use getQuery() or the PSR-7 getQueryParams() and withQueryParams() methods instead. See https://book.cakephp.org/3/en/appendices/3-4-migration-guide.html
@cblanken cblanken marked this pull request as draft April 22, 2024 01:22
@jiru
Copy link
Member

jiru commented Apr 22, 2024

What an ambitious PR! 😮 Thank you for getting on with this task.

I assume named parameters are still in use for some routes

Named parameters used to be the default for CakePHP < 3. They were used on Tatoeba for every page before. For example, if you look at this old archived page, you can see the "next page" links have a /page:2 parameter (as opposed to ?page=2). CakePHP 3.x changed that default, we also did, but we did not want to break existing links, so just to keep backward compatibility we use parseNamedParams(). You can try opening https://tatoeba.org/fr/sentences_lists/show/724/page:2 and you’ll see it still brings you to page 2 (a redirect would have been better though 😔). That’s the only reason.

@cblanken
Copy link
Contributor Author

cblanken commented Apr 25, 2024

(a redirect would have been better though 😔). That’s the only reason.

Do you think it'd be best to inline the parseNamedParams() function from CakePHP 3.x? Otherwise this might be a good opportunity to stop supporting named parameters. If someone with access to the server could check the logs, there may not even be any recent usage of links with named parameters. In that case it'd probably be best to remove completely since they won't be supported by CakePHP in the future.

@jiru
Copy link
Member

jiru commented Apr 29, 2024

As far as I know we only keep logs for a short time so they probably won't give a good overview of old links usage. To keep maintaining old links or not to and potentially break things, that is the question! Personally I'm in favor of keeping old links working as long as it's not a burden (which I don't think it is, I mean it's fine to keep our own copy of parseNamedParameters()). I don't think anybody likes getting 404 errors when browsing the web.

cblanken added 14 commits May 3, 2024 10:26
> EventManager::attach() is deprecated. Use EventManager::on() instead.
See https://api.cakephp.org/3.0/class-Cake.Event.EventManager.html
> Router::parseNamedParams() is deprecated. 2.x backwards compatible named parameter support will be removed in 4.0
To continue to support legacy URLs using named parameters, the Router::parseNamedParams()
method has been inlined into the AppController with the deprecation warning removed.
Fix the following deprecation(s)
> Helper::$request is deprecated. Use $helper->getView()->getRequest() instead.
See https://book.cakephp.org/3/en/appendices/3-7-migration-guide.html#deprecations
> The ArrayAccess methods will be removed in 4.0.0.Use withParam() instead
See https://book.cakephp.org/3/en/appendices/3-4-migration-guide.html
> Accessing `data` as a property will be removed in 4.0.0. Use request->getData() instead.
See https://book.cakephp.org/3/en/appendices/3-4-migration-guide.html

Note: in a couple instances, an `isset()` call is replaced with with a `null` check
since `isset()` cannot be used on the result of an expression.
> ViewBuilder::autoLayout() is deprecated. Use ViewBuilder::enableAutoLayout() or ViewBuilder::isAutoLayoutEnable() instead.
See https://book.cakephp.org/3/en/appendices/3-4-migration-guide.html#deprecated-combined-get-set-methods
> Plugin::load() is deprecated. Use Application::addPlugin() instead. This method will be removed in 4.0.0
See https://book.cakephp.org/3/en/appendices/3-7-migration-guide.html#testsuite
Fix the following deprecation warnings:

> ValidatorAwareTrait::validator() is deprecated. Use ValidatorAwareTrait::getValidator()/setValidator() instead.
See https://book.cakephp.org/3/en/appendices/3-5-migration-guide.html#validation

> App\Model\Entity\Export::errors() is deprecated. Use setError()/getError() or setErrors()/getErrors() instead.
Missing from the migration guides but deprecated on the EntityInterface in 3.4.0 according to the source.
See https://github.com/cakephp/cakephp/blob/571fb867995ccfa4034ec5b66577f0ab2acd4429/src/Datasource/EntityInterface.php#L188

> Validator::errors() is deprecated. Renamed to Validator::validate().
See https://book.cakephp.org/3/en/appendices/3-9-migration-guide.html#deprecations

> App\Model\Table\ExportsTable::schema() is deprecated. Use setSchema()/getSchema() instead.
See https://book.cakephp.org/3/en/appendices/3-4-migration-guide.html#deprecated-combined-get-set-methods
> Email::setTemplate() is deprecated. Use $email->viewBuilder()->setTemplate() instead.
See https://book.cakephp.org/3/en/appendices/3-7-migration-guide.html#deprecations
> Response::header() is deprecated. Use `withHeader()`, `getHeaderLine()` and `getHeaders()` instead.
See https://book.cakephp.org/3/en/appendices/3-4-migration-guide.html#request-response-deprecations
> Declaring fixtures in underscored format in TestCase::$fixtures is deprecated.
Upper CamelCase/PascalCase is expected instead.
See https://book.cakephp.org/3/en/appendices/3-7-migration-guide.html#deprecations
This one was missed on my first pass. See commit 30b3b9c for other instances.
> The ArrayAccess methods will be removed in 4.0.0. Use getParam() instead.
See https://book.cakephp.org/3/en/appendices/3-4-migration-guide.html#request-response-deprecations
> Helper::$request is deprecated. Use $helper->getView()->getRequest() instead.
See https://book.cakephp.org/3/en/appendices/3-7-migration-guide.html#deprecations
See commit b52e47d for other instances
Fix the following deprecations
> App\Model\Entity\Sentence::dirty() is deprecated. Use setDirty()/isDirty() instead
> App\Model\Entity\Vocable::dirty() is deprecated. Use setDirty()/isDirty() instead
> App\Model\Entity\Sentence::errors() is deprecated. Use setError()/getError() or setErrors()/getErrors() instead.
@cblanken
Copy link
Contributor Author

cblanken commented Oct 8, 2024

Okay, I've done what I can with this, but I don't think I'm familiar enough with CakePHP to sort out what's remaining. I've documented the changes as well as I could with links to the appropriate migration guide pages.

Maybe I'll return to it once I've worked with the codebase a bit more, but for the time being, anyone else who wants to pick this up is welcome to it.

Note

When advancing the application to 4.x with the upgrade tool, be aware that PHP 8.x is required to run the tool, even though 4.x applications only have a minimum requirement of PHP 7.2. For more details, see https://book.cakephp.org/4/en/appendices/4-0-upgrade-guide.html#upgrade-to-php-8-0.

@jiru
Copy link
Member

jiru commented Oct 12, 2024

Thank you very much for working on this, @cblanken!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants