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

Preload: Don't try to preload private pages #5885

Closed
webtrainingwheels opened this issue Apr 21, 2023 · 3 comments · Fixed by #5920
Closed

Preload: Don't try to preload private pages #5885

webtrainingwheels opened this issue Apr 21, 2023 · 3 comments · Fixed by #5920
Assignees
Labels
effort: [S] 1-2 days of estimated development time module: preload priority: medium Issues which are important, but no one will go out of business. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@webtrainingwheels
Copy link

Is your feature request related to a problem? Please describe.
WP Rocket will try to preload pages that have been set to Private in WordPress. This generates unnecessary requests and results in 404 errors. If there are a lot of them, it would also slow down the whole process.

Describe the solution you'd like
We could check if a page is private, and if so, automatically exclude it from Preload.

Describe alternatives you've considered
Customers could manually exclude these URLs, but that's not ideal from a UX point of view.

Additional context
Related ticket: https://secure.helpscout.net/conversation/2219018645/415412?folderId=377611

@piotrbak
Copy link
Contributor

piotrbak commented Apr 25, 2023

To reproduce:

  1. Enable Preload
  2. Go to the Add new page section (or update the private page)
  3. Switch the Visibility to Private
  4. Observe access.log

Additional context
The preload happens only once, after it it's not added to the cache table

Acceptance Criteria:

  1. The URL should not be preloaded after being added as private nor when the private page is updated
  2. Regular, public URLs should still be preloaded and added to cache table after being published or updated
  3. The same should apply to all post types
  4. To clarify the 1st point, when editing the public post and setting it as private, the request should not happen either

@piotrbak piotrbak added type: enhancement Improvements that slightly enhance existing functionality and are fast to implement module: preload priority: medium Issues which are important, but no one will go out of business. labels Apr 25, 2023
@webtrainingwheels
Copy link
Author

webtrainingwheels commented Apr 25, 2023

The preload happens only once, after it it's not added to the cache table

Just to add, it happens once per each time that preloading is triggered. i.e so we'll also try again when the page is updated.

@jeawhanlee jeawhanlee added GROOMING IN PROGRESS Use this label when the issue is currently being groomed. and removed needs: grooming labels May 9, 2023
@jeawhanlee
Copy link
Contributor

jeawhanlee commented May 9, 2023

Reproduce the problem ✅

I was able to reproduce the problem.

Identify the root cause ✅

We are not checking post status before adding urls to be preloaded in the cache table.

Scope a solution ✅

We can create a new method in https://github.com/wp-media/wp-rocket/blob/fbfdfed79ca924284394fa53f84bec7b61413587/inc/Engine/Preload/Controller/ClearCache.php

is_private( $url ) {
    $post_id = url_to_postid( $url );
    return 'private' == get_post_status( $post_id );
}

Then use new method in

public function partial_clean( array $urls ) {

if ( is_private( $url ) ) {
    continue;
}

Update tests too.

Estimate the effort ✅

[S]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [S] 1-2 days of estimated development time module: preload priority: medium Issues which are important, but no one will go out of business. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants