Added logic to clean root files

[CrochetFeve0251]

Description

Clean root files when cleaning WPR cache.

Fixes #5611

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

Is the solution different from the one proposed during the grooming?

No.

How Has This Been Tested?

• Automated test
• Local test

Checklist:

Please delete the options that are not relevant.

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[vmanthos]

@CrochetFeve0251 We are only clearing files, i.e. not folders that exist in the root folder.

If:

1. there are any URLs that can still be accessed without having the language dir, e.g. en or fr, in the path, or
2. after enabling WMPL's Use directory for default language (in WPML > Languages) for the first time, any pre-existing folders in the root directory will not be cleared.

Visiting those URLs will serve cached content given that cache for them exists.

So, even when using rocket_clean_domain(), e.g. when clearing the whole cache using the admin bar menu item, there will still be folders in /cache/wp-rocket/example.com/. As far as I can tell, we should be clearing all files and folders there when the whole cache is cleared.

When clearing language-specific cache, e.g. that of English, we should clear every file and folder in the root directory except for the folders of other languages, e.g. /fr/, /ar/, etc.

The same should happen when we clear a post's cache. In this case, we shouldn't delete the whole /en/ folder. Just the related URLs.

@piotrbak What do you think?

[piotrbak]

@vmanthos If we're leaving only folders, not cached files, I believe this is acceptable side effect. It doesn't bring any defects in functionality at all.

What do you think?

[vmanthos]

@piotrbak With the following steps we'll get stale cached content:

1. Disable WPML's "Use directory for default language" option.
2. Clear and Preload WP Rocket's cache.
3. Once done, enable WPML's feature.
4. Previously cached URLs will be accessible by the incorrect URL.
   This will be a problem for those who either have bookmarked URLs or are manually typing them.

Preconditions: Apache server and our rewrite rules in place.

The result of the offered steps will be the norm for anyone using WP Rocket and then switching on WPML's "Use directory for default language" option.

@CrochetFeve0251 suggested clearing the cache when WPML's option is enabled/disabled which is a good solution. The question at hand is if that should be done for the whole cache, clearing even the cache of URLs whose permalinks haven't changed, or trying to clear the cache just for the affected URLs.

[piotrbak]

I think we can clear the whole cache. That kind of change is not a frequent one

[vmanthos]

@CrochetFeve0251 Thank you for the PR. Please find my notes below:

1. When disabling the "Use directory for default language" option the cache is not cleared. In this case, people visiting URLs having the default language in the path will see "old" cached content. The cache is cleared when enabling the option.

2. Also, I'm getting the following fatal error whenever rocket_clean_domain() runs, e.g. when clearing and preloading the cache for all languages using the admin bar menu item, when saving WP Rocket's settings, switching the theme, etc :

PHP Fatal error:  Uncaught TypeError: array_keys(): Argument #1 ($array) must be of type array, bool given in /shared/httpd/wprocketest/htdocs/wp-content/plugins/wp-rocket/inc/ThirdParty/Plugins/I18n/WPML.php:135
Stack trace:
#0 /shared/httpd/wprocketest/htdocs/wp-content/plugins/wp-rocket/inc/ThirdParty/Plugins/I18n/WPML.php(135): array_keys(false)
#1 /shared/httpd/wprocketest/htdocs/wp-includes/class-wp-hook.php(310): WP_Rocket\ThirdParty\Plugins\I18n\WPML->remove_root_cached_files('/shared/httpd/w...')
#2 /shared/httpd/wprocketest/htdocs/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters(NULL, Array)
#3 /shared/httpd/wprocketest/htdocs/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#4 /shared/httpd/wprocketest/htdocs/wp-content/plugins/wp-rocket/inc/functions/files.php(806): do_action('after_rocket_cl...', '/shared/httpd/w...', '', 'https://wprocke...')
#5 /shared/httpd/wprocketest/htdocs/wp-content/plugins/wp-rocket/inc/common/purge.php(438): rocket_clean_domain('')
#6 /shared/httpd/wprocketest/htdocs/wp-includes/class-wp-hook.php(308): do_admin_post_rocket_purge_cache('')
#7 /shared/httpd/wprocketest/htdocs/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters('', Array)
#8 /shared/httpd/wprocketest/htdocs/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#9 /shared/httpd/wprocketest/htdocs/wp-admin/admin-post.php(85): do_action('admin_post_purg...')
#10 {main}
  thrown in /shared/httpd/wprocketest/htdocs/wp-content/plugins/wp-rocket/inc/ThirdParty/Plugins/I18n/WPML.php on line 135

3. Clicking the "save" button in WPML language settings clears the whole cache even if the settings haven't changed:
   

Issues 2 & 3 happen when the following snippet is used:

add_action( 'after_rocket_clean_domain', function(){
	error_log( " run\n", 3, ABSPATH . 'RCD.log');
});

Can you please look into these? 🙏

[vmanthos]

@CrochetFeve0251 All the issues I mentioned in the previous comment are resolved. Thank you!

@piotrbak I noticed that URLs following the previous permalinks scheme are still present in the wpr_rocket_cache table.

Visiting such a URL, e.g. https://example.com/post-1, triggers a 301 redirect to https://example.com/en/post-1 when visited in the browser. The same happens with preload. Such URLs are marked as failed. We'll retry to preload those when rocket_preload_revert_old_failed_rows fires.

On top of clearing the cache when enabling/disabling the WPML feature, I believe we should remove the existing URLs from the cache table and re-initialize the process to add them. What do you think?

[piotrbak]

On top of clearing the cache when enabling/disabling the WPML feature, I believe we should remove the existing URLs from the cache table and re-initialize the process to add them. What do you think?

That would happen when changing the way how WPML is serving the languages only, right?

[vmanthos]

That would happen when changing the way how WPML is serving the languages only, right?

Exactly! Only when enabling/disabling the "Use directory for default language" option.

[piotrbak]

Yes, we should do that.

[vmanthos]

@CrochetFeve0251 The language-specific folders, e.g. /en, /fr, are cleared now when WPML is deactivated/activated while the "Use directory for default language" option is enabled. 👍

However, any folders existing in cache/wp-rocket/example.com/ will not be removed. Because of this, if /hello-world was cached before enabling WPML it will be accessible from these URLs:

1. https://example.com/hello-world
2. https://example.com/en/hello-world/

and won't be removed even after clearing the cache for all languages using the admin bar menu.

Steps to reproduce these:

1. In WPML, enable the "Use directory for default language" option.
2. After the cache for translated posts has been created deactivate the plugin.
3. Visit /hello-world.
4. Enable WPML, and edit /hello-world.
5. Repeat step 3 and check if the additions from step 4 are there.

I believe we should remove all folders when:

1. WMPL (sitepress-multilingual-cms/sitepress.php) is activated AND the the "Use directory for default language" option is enabled.
2. When WMPL (sitepress-multilingual-cms/sitepress.php) is deactivated AND the the "Use directory for default language" option is enabled.

What do you think @piotrbak?

[vmanthos]

@CrochetFeve0251 The cache is cleared when enabling/disabling WPML while "Use directory for default language" is enabled. 👍

I noticed the following related to preload while SEOPress was installed.

When:

1. Enabling WPML ("Use directory for default language" option already enabled) > the cache is cleared and the sitemap is preloaded.
2. If, while there are still pending jobs, we deactivate WPML > the cache is cleared and entries are removed from the wpr_rocket_cache table but the sitemap is not preloaded.

I debugged this and I can see that in step 2 we are getting URLs of sitemaps that don't exist which explains why URLs are not preloaded. Those URLs were the ones that correspond to those when WPML was enabled.

Something similar happens with the default WordPress sitemap as well when disabling the "Use directory for default language" option.

We only preload the additional language sitemap, e.g. for fr it's https://example.com/fr/wp-sitemap.xml, while the one for the main language doesn't exist, e.g. for en it's https://example.com/en/wp-sitemap.xml. It should be https://example.com/wp-sitemap.xml

I'm not sure if this is an issue of the WPML + Preload combination and if it can be resolved by delaying the fetching of sitemaps.

Unless @piotrbak thinks otherwise would you mind taking a look? 🙏

[piotrbak]

@vmanthos @CrochetFeve0251 Let's see the root cause and if there's a very easy workaround. If not, we can treat it as an edge case as disabling WPML is not something customers will be doing often.

[CrochetFeve0251]

@vmanthos @CrochetFeve0251 Let's see the root cause and if there's a very easy workaround. If not, we can treat it as an edge case as disabling WPML is not something customers will be doing often.

I think I maybe got an idea why. As the links stucture is change there is a huge change permalink_structure_changed is fired by WPML and there is another chance that the sitemap.xml are created at that moment making the fetch fail.

If I am not wrong we already implemented a solution we can reuse here.

[vmanthos]

Thank you for the continuous effort @CrochetFeve0251!

This is working as expected! 👍

TestRail report: testrail-report-497.pdf