Feature RTC Maintainers Checked PR-5.0-dev PR-5.1-dev Pending

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
16 Mar 2023

Summary of Changes

We are migrating away from the Joomla\CMS\Filesystem classes to the framework Filesystem classes. This PR replaces all occurences of Joomla\CMS\Filesystem\Folder::exists() with is_dir(). The former method does not exist in the framework class, mainly because it is an unnecessary wrapper for the later. Where necessary, an import for Joomla\Filesystem\Path has been added and unnecessary ones removed. All the existing imports to Joomla\CMS\Filesystem\Path are changed to the framework package in another PR.

Joomla\Filesystem\Path::clean() should be used when the given path can not be 100% assumed to be safe, for example when the path is coming from some outside input. That method will make sure that a path will have the systems directory separator and not contain double slashes or double backslashes.

Testing Instructions

Codereview

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar joomla-cms-bot joomla-cms-bot - change - 16 Mar 2023
Category Administration com_admin com_config com_menus com_templates Libraries Front End Plugins
avatar Hackwar Hackwar - open - 16 Mar 2023
avatar Hackwar Hackwar - change - 16 Mar 2023
Status New Pending
avatar laoneo
laoneo - comment - 19 Mar 2023

It would be helpful for extension developers when you can write a couple of words in the pr description why you have used Path::clean.

avatar Hackwar Hackwar - change - 19 Mar 2023
The description was changed
avatar Hackwar Hackwar - edited - 19 Mar 2023
avatar Hackwar
Hackwar - comment - 19 Mar 2023

Done.

avatar Hackwar Hackwar - change - 19 Mar 2023
Labels Added: PR-4.4-dev
avatar laoneo laoneo - change - 14 Apr 2023
Title
[4.4] Migrating Joomla\CMS\Filesystem\Folder::exists() to is_dir()
[5.0] Migrating Joomla\CMS\Filesystem\Folder::exists() to is_dir()
avatar laoneo laoneo - edited - 14 Apr 2023
avatar Hackwar Hackwar - change - 17 Apr 2023
Labels Added: bug PR-5.0-dev
Removed: PR-4.4-dev
avatar Hackwar Hackwar - change - 18 Apr 2023
Labels Added: b/c break
avatar Hackwar Hackwar - reopen - 18 Apr 2023
avatar HLeithner
HLeithner - comment - 2 Sep 2023

I fixed the merge conflict but I'm not 100% sure if it make sense, since now we call is_dir(Path::clean()) before we just call Folder::exists() which implicit called Path::clean() for 3rd party developer it's possible that they forget or ignore the Path::clean() which might be an issue?

avatar Hackwar
Hackwar - comment - 2 Sep 2023

The nice thing is, that we are not changing third party code, but just our core code. After this change, third party code will still behave the same. ?

avatar Hackwar
Hackwar - comment - 2 Sep 2023

The Codestyle error reported here is unrelated to this PR.

avatar HLeithner HLeithner - change - 5 Oct 2023
Title
[5.0] Migrating Joomla\CMS\Filesystem\Folder::exists() to is_dir()
[5.1] Migrating Joomla\CMS\Filesystem\Folder::exists() to is_dir()
avatar HLeithner HLeithner - edited - 5 Oct 2023
avatar Hackwar Hackwar - change - 3 Nov 2023
Labels Added: Feature Maintainers Checked PR-5.1-dev
Removed: bug
avatar Hackwar Hackwar - change - 25 Dec 2023
Labels Removed: b/c break
avatar Fedik Fedik - test_item - 25 Feb 2024 - Tested successfully
avatar Fedik
Fedik - comment - 25 Feb 2024

I have tested this item ✅ successfully on 58f6055


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40131.

avatar bembelimen bembelimen - change - 2 Mar 2024
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2024-03-02 06:38:15
Closed_By bembelimen
Labels Added: RTC
avatar bembelimen bembelimen - close - 2 Mar 2024
avatar bembelimen bembelimen - merge - 2 Mar 2024
avatar bembelimen
bembelimen - comment - 2 Mar 2024

Thx

Add a Comment

Login with GitHub to post a comment