wp-rocket icon indicating copy to clipboard operation
wp-rocket copied to clipboard

Elementor templates clearing cache in full

Open DahmaniAdame opened this issue 2 years ago • 25 comments

Before submitting an issue please check that you’ve completed the following steps:

  • [x] Made sure you’re on the latest version 3.13
  • [x] Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug

In that specific case, it's the popup module. But the same logic should apply to all Elementor's templates.

Templates don't have a permalink structure.

Links look like:

http://domain.ext/?elementor_library=free-delivery-information-popup-1

When these popups are updated and rocket_clean_post is triggered, we extract the path to the root cache folder.

Array
(
    [host] => domain.ext
    [path] => /
    [scheme] => http
    [query] => elementor_library=some-popup-title
    [fragment] => 
)

And we end up purging /public_html/wp-content/cache/wp-rocket/domain.ext/

https://github.com/wp-media/wp-rocket/blob/1f8dfc9660a112a2a937e2fb90e51cad0b9572a5/inc/functions/files.php#L590-L600

rocket_clean_files should be safeguarded against processing that kind of URLs.

We could possibly add an additional check here:

https://github.com/wp-media/wp-rocket/blob/1f8dfc9660a112a2a937e2fb90e51cad0b9572a5/inc/functions/files.php#L588

To see if $parsed_url['path'] !== '/'.

In addition to that, ideally, we shouldn't pick up the post permalink when dealing with Elementor's templates.

Instead, we should restrict cache purging to posts where the template, in this example, a popup, is being used.

Assuming that the template post id is 12345

The element will be referenced as part of the _elementor_data custom field when being used on a page.

Inside these data, an entry like the following will be found. Here is an example for popups:

[elementor-tag id=\"42cdb83\" name=\"popup\" settings=\"%7B%22popup%22%3A%2212345%22%7D\"]"}}]

We can have a SQL query with the template ID to check the wp_postmeta table to collect all related posts, and purge them from cache.

We could maybe have a new function to replace rocket_clean_post as the hook to process Elementor templates cache clearing to not over complicate rocket_clean_post logic?

To Reproduce Steps to reproduce the behavior:

  1. Use Elementor's popups
  2. Buildup cache
  3. Update the popup post
  4. See error

Expected behavior We shouldn't clear the cache in full in that context.

Screenshots N/A

Additional context Ticket - https://secure.helpscout.net/conversation/2179456687/407285/ Issue temporarily fixable using the following helper plugin - https://drive.google.com/file/d/1DAmftPxPWgld3MEyXPWMhmukOqwzsi98/view?usp=sharing

Backlog Grooming (for WP Media dev team use only)

  • [x] Reproduce the problem
  • [x] Identify the root cause
  • [x] Scope a solution
  • [x] Estimate the effort

DahmaniAdame avatar Mar 31 '23 08:03 DahmaniAdame

Acceptance Criteria

  • rocket_clean_post should be guarded against clearing the whole cache (possible relation https://github.com/wp-media/wp-rocket/issues/5887)
  • Clear the cache of posts that contain updated Elementor module
  • No regressions in rocket_clean_post when updating the regular posts

piotrbak avatar May 06 '23 17:05 piotrbak

Reproduce the problem ✅

I was able to reproduce the problem.

Identify the root cause ✅

Just like @DahmaniAdame Stated

Scope a solution ✅

To stop full cache when updating a template in elementor, we can do a check in https://github.com/wp-media/wp-rocket/blob/1f8dfc9660a112a2a937e2fb90e51cad0b9572a5/inc/functions/files.php#L587 like @DahmaniAdame stated

if ( '/' === $parsed_url['path'] || '' === $parsed_url['path'] ) {
  continue;
}

To remove associated posts when elementor modules are updated are 2 ways

  • For Templates that are used directly in posts, elementor saves meta_key relative to that post like @DahmaniAdame described. We will add a new event - elementor/editor/after_save in the elementor 3rd party class https://github.com/wp-media/wp-rocket/blob/1f8dfc9660a112a2a937e2fb90e51cad0b9572a5/inc/ThirdParty/Plugins/PageBuilder/Elementor.php#L53 and create a callback method: clear_related_post_cache( $post_id ) In this method we will then use the $post_id to query the postmeta table to return rows with $post_id in meta_value column and clear cache of posts with rocket_clean_post

  • For Templates with conditions, elementor saves this as _elementor_conditions in the postmeta table with various value(not all are included):

    • include/general - entire site
    • include/singular - pretty much like entire site.
    • include/singular/front_page - only front page/homepage
    • include/singular/post - all posts
    • include/singular/page - all pages
    • include/singular/post/9 - specific post with post_id as 9
    • include/singular/page/2 - specific page with post_id as 2
    • include/singular/not_found404 - 404 page

We can do checks against this values and clear as expected. Add tests too.

Estimate the effort ✅

[S]

@engahmeds3ed What do you think?

jeawhanlee avatar May 12 '23 12:05 jeawhanlee

@jeawhanlee @engahmeds3ed The effort here is [M], not [S], right?

piotrbak avatar May 21 '23 14:05 piotrbak

@piotrbak can we add clearing the Used CSS as a requirement for this one? Or should I open a separate issue for it?

DahmaniAdame avatar Jun 23 '23 08:06 DahmaniAdame

@DahmaniAdame Just to be sure, it's about clearing the Used CSS of related posts, right?

piotrbak avatar Jun 23 '23 10:06 piotrbak

@piotrbak yes, exactly. If the template is edited, the style will change. What will happen is that we will be shipping the wrong style on related posts if we just clear cache. Both cache and Used CSS should be cleared.

DahmaniAdame avatar Jun 23 '23 10:06 DahmaniAdame

@piotrbak Can you also provide the ability to disable cache clearing when templates are edited? In my case, I have many templates that affect only one page. The site has over 1000 pages, and every time the editor makes changes to the text, the entire cache is cleared.

valmirselmani avatar Jul 04 '23 11:07 valmirselmani

@piotrbak It's the same issue. When we edit Elementor templates, the cache is cleared in full. I just like to have the possibility to disable clearing the cache in full with a filter when templates are edited.

valmirselmani avatar Jul 17 '23 10:07 valmirselmani

@DahmaniAdame I think this should be limited to specific templates only. As per @wp-media/qa and I also confirmed this on my website, if you edit the regular template using free Elementor, the changes are not applied to the pages that contain this template. Steps to reproduce :

  1. Create a template, Templates > Add new section template
  2. Add it to the page
  3. Edit the template
  4. Check the page, the template is not updated

The templates that are auto-updating are for sure:

  1. Popups
  2. Footers
  3. Headers

piotrbak avatar Jul 17 '23 10:07 piotrbak

@valmirselmani Yes, I can see that. For now you could modify the following helper plugin to achieve what you want: https://github.com/wp-media/wp-rocket-helpers/blob/master/cache/wp-rocket-no-cache-auto-purge/wp-rocket-no-cache-auto-purge.php

piotrbak avatar Jul 17 '23 10:07 piotrbak

@piotrbak

the template is not updated

The regular templates are like partials to be added to pages. They are not dynamic, as far as I know. Updating a template won't update the page where it was inserted.

It's not expected that the page's template content will change if the template is update. And we are not expected to clear the page's cache if the template is updated.

I think this should be limited to specific templates only

I believe it already is. It's supposed to target only templates adding entries to the post's _elementor_data.

@valmirselmani current issue is what this ticket is about. WP Rocket converts the template's path to become / and causes cache to be cleared in full.

This is what this part will fix - https://github.com/wp-media/wp-rocket/blob/457d4c5cd23c6d6435c17cba4c36adea9c247e36/inc/functions/files.php#L590-L592

@valmirselmani for now, using the helper plugin to disable automatic cache clearing as advised by Piotr should help.

DahmaniAdame avatar Jul 18 '23 08:07 DahmaniAdame

@DahmaniAdame The issue initially was about the all templates, was surprised while checking the QA report + doing some research 😁

Sending it back to QA then. Do you have a list of templates that are triggering the cache clearing or it should be explored?

piotrbak avatar Jul 18 '23 09:07 piotrbak

@piotrbak the first part, where all cache is cleared, is about all templates!

So, what needs to be checked by @wp-media/qa is if cache is still fully clear if any template updated.

It's the second part where specific posts are cleared related to dynamic template, let's call them that, that is specific to the part where we look for _elementor_data.

For this one, what needs to be checked is if all posts with the specific popup for example added are the only ones being cleared.

Do you have a list of templates that are triggering the cache clearing or it should be explored?

Unfortunately, no.

I tested with Popups initially. But Header/Footer should have the same logic.

Add-ons with popups or handling at least these 3 types are likely piggybacking on that logic as well.

Basically, whatever adds a template element with an entry on the post's _elementor_data would qualify for the fix.

DahmaniAdame avatar Jul 18 '23 09:07 DahmaniAdame

@piotrbak @DahmaniAdame I already have this https://github.com/wp-media/wp-rocket-helpers/blob/master/cache/wp-rocket-no-cache-auto-purge/wp-rocket-no-cache-auto-purge.php in place, but it still clears the cache in full.

valmirselmani avatar Jul 18 '23 12:07 valmirselmani

@valmirselmani can you open a ticket for someone to have a closer look?

DahmaniAdame avatar Jul 18 '23 12:07 DahmaniAdame

I'm blocking this one for now, it needs a product discussion a bit, will move it back to grooming as soon as we have all the answers.

piotrbak avatar Jul 20 '23 15:07 piotrbak

@DahmaniAdame This issue became more complicated than we thought it'll be. To avoid releasing important regressions, I'd suggest adding to AC:

  1. Clear the whole cache when display conditions are changed within the template
  2. If the template is used site-wide and it's updated, we should clear the whole cache (but not Used CSS, as the template could have been updated jsut by changing the text)

Also, we should preserve the current functionality already introduced in this PR:

  1. rocket_clean_post should be guarded against clearing the whole cache
  2. Clear the cache and used CSS of posts that contain updated Elementor module
  3. No regressions in rocket_clean_post when updating the regular posts

If we introduce the above, there'll be regression related to not clearing cache of the Templates with Conditions, as per @jeawhanlee: - include/singular - pretty much like entire site. - include/singular/front_page - only front page/homepage - include/singular/post - all posts - include/singular/page - all pages - include/singular/post/9 - specific post with post_id as 9 - include/singular/page/2 - specific page with post_id as 2 - include/singular/not_found404 - 404 page

Handling all those scenarios with cache clearing will be complex and time-consuming, on the other hand, right now when saving the template we're clearing everything.

piotrbak avatar Jul 20 '23 15:07 piotrbak

@piotrbak let's go with that.

If the template is used site-wide and it's updated, we should clear the whole cache (but not Used CSS, as the template could have been updated jsut by changing the text)

Do we have a way to know if it's just text or get Elementor involved to provide a way to do so?

If yes, we can use that to clear the Used CSS selectively if not just text is changed.

If not, we should go with clearing the Used CSS to avoid risks of displaying a broken page.

DahmaniAdame avatar Jul 25 '23 05:07 DahmaniAdame

@DahmaniAdame Are we okay with this part?

If we introduce the above, there'll be regression related to not clearing cache of the Templates with Conditions, https://github.com/wp-media/wp-rocket/issues/5848#issuecomment-1545701597

We could either:

  1. Automatically clear cache + Used CSS sitewide
  2. Display a notice telling customer about the update, so he's aware of the change and let him decide what to do
  3. Try to find a way to make a connection between conditions and URLs in cache, which will be time consuming and might break in the future if Elementor changes anything in the logic

piotrbak avatar Jul 25 '23 15:07 piotrbak

@piotrbak

Are we okay with this part?

Yes. That's acceptable.

Can we involve Elementor and see if they can offer a hook for caching plugins to handle that part?

Otherwise, option 2 looks like a good middle ground. 1 might cause an unnecessary cache/RUCSS cycle. And 3 will be high maintenance.

DahmaniAdame avatar Jul 26 '23 07:07 DahmaniAdame

To follow-up on this one, with new acceptance criteria. We'll preserve the current functionality of this PR:

  1. rocket_clean_post should be guarded against clearing the whole cache :heavy_check_mark:
  2. Clear the cache and used CSS (if it's enabled) of posts that contain updated Elementor module :heavy_check_mark:
  3. No regressions in rocket_clean_post when updating the regular posts :heavy_check_mark:

And for the second scenario, with the condition templates:

  1. Whenever Elementor Condition Template is updated and Used CSS is enabled, we'll display a dismissible warning on all admin pages :heavy_check_mark:
  2. Whenever Elementor Condition Template is updated and Used CSS is disabled, notice will not be displayed. :heavy_check_mark:
  3. Wording will be as following: WP Rocket: Your Elementor template was updated. Clear the Used CSS if the layout, design or CSS styles were changed. :heavy_check_mark:
  4. After clicking on Clear Used CSS, we'll purge the Used CSS and Cache :heavy_check_mark:
  5. After clicking on Dismiss, the message will disappear :heavy_check_mark:
  6. If the Elementor Condition Template will be updated in the future, the message should be displayed once again :heavy_check_mark:

piotrbak avatar Jul 31 '23 11:07 piotrbak

Reproduce the problem

I saw @jeawhanlee was able to reproduce the problem on the first grooming and explained the root cause.

Scope a solution

The solution we gonna have here will have to reuse as much as possible. For guarding against clearing the whole cache we will reused that logic.

'rocket_pre_clean_post' => [ 'exclude_post_type_cache_clearing', 10, 2 ],
/**
	 * Exclude elementor library post type from cache clearing.
	 *
	 * @param mixed $clear_post A preemptive return value. Default null.
	 * @param Post  $post Post Object.
	 * @return boolean
	 */
	public function exclude_post_type_cache_clearing( $clear_post, $post ) {
		if ( 'elementor_library' === $post->post_type ) {
			return true;
		}

		return $clear_post;
	}

Then concerning the modal we will have to first handle the logic that will detect a change on the elementor template. For that I propose to create a second callback on update_elementor_library_metadata that will setup a transient.

function setup_transient($check, $object_id, $meta_key, $meta_value, $prev_value) {
   if('_elementor_conditions' !== $meta_key || $meta_value === $prev_value || ! $this->options->get('', false)) {
     return;
   }
   set_transient('wpr_elementor_need_purge', true);
   $boxes = get_user_meta( get_current_user_id(), 'rocket_boxes', true );
   unset($boxes['maybe_clear_cache_change_notice'']);
}

Once we done that then we have to handle the notice by adding the following callback on admin_notices:

public function maybe_clear_cache_change_notice() {

		$boxes = get_user_meta( get_current_user_id(), 'rocket_boxes', true );

		if ( in_array( __FUNCTION__, (array) $boxes, true ) ) {
			return;
		}

		if ( ! current_user_can( 'rocket_manage_options' ) ) {
			return;
		}

		$notice = get_transient( 'wpr_elementor_need_purge' );

		if ( ! $notice ) {
			return;
		}

		$args = [
			'status'         => 'warning',
			'dismissible'    => '',
			'dismiss_button'         => __FUNCTION__,
			'message'        => sprintf(
			// translators: %1$s = <strong>, %2$s = </strong>, %3$s = <a>, %4$s = </a>.
				__( '%1$sWP Rocket:%2$s Your Elementor template was updated. Clear the Used CSS if the layout, design or CSS styles were changed.', 'rocket' ),
				'<strong>',
				'</strong>',
				'</a>'
			),
			'action'         => 'clear_cache',
		];

		rocket_notice_html( $args );
	}

Also we need to handle the clearing from related posts when an Elementor module is cleared. For that we need to hook the following callback to elementor/editor/after_save:

public function clear_related_post_cache( int $post_id ) {
		global $wpdb;

		$template_id = '%' . $wpdb->esc_like( $post_id ) . '%';

		// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching
		$results = $wpdb->get_results(
				$wpdb->prepare(
				"SELECT post_id FROM `$wpdb->postmeta` WHERE `meta_key` = %s AND `meta_value` LIKE %s",
				[ '_elementor_data', $template_id ]
				)
			);

		if ( ! $results ) {
			return;
		}

		foreach ( $results as $result ) {
			if ( 'publish' === get_post_status( $result->post_id ) ) {
				rocket_clean_post( $result->post_id );

				if ( $this->options->get( 'remove_unused_css', 0 ) ) {
					$url = get_permalink( $result->post_id );
					$this->used_css->delete_used_css( $url );
				}
			}
		}
	}

Estimate the effort

Effort S

CrochetFeve0251 avatar Aug 07 '23 15:08 CrochetFeve0251

@piotrbak I would need a elementor pro license to test the implmentation the old one look like expired

CrochetFeve0251 avatar Sep 08 '23 08:09 CrochetFeve0251

For the value check with the previous values. In the current state it will be always different as the previous value always return ''. However Elementor handle that case from the UI and prevent the user from saving.

CrochetFeve0251 avatar Sep 08 '23 12:09 CrochetFeve0251

Related case here: https://secure.helpscout.net/conversation/2481477875/469222?folderId=2675957 Slack thread: https://wp-media.slack.com/archives/C43T1AYMQ/p1705678445916809 URL passed to partial cache purge: https://staging1.ksmedizintechnik.de/?cms_block=home-kw51 Fixed with a helper plugin by Alfonso.

camilamadronero avatar Jan 22 '24 20:01 camilamadronero

another case here: https://secure.helpscout.net/conversation/3052335730/582978?viewId=2683093

alfonso100 avatar Aug 29 '25 16:08 alfonso100