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

Fix problems with placing object-cache.php drop-in #672

Merged
merged 5 commits into from
Mar 15, 2023

Conversation

felixarntz
Copy link
Member

@felixarntz felixarntz commented Mar 14, 2023

Summary

Fixes #630

Relevant technical choices

  • So far, if there was an existing object-cache.php drop-in, the plugin would attempt to rename it to object-cache-plst-orig.php, then place its own object-cache.php file which would then also require_once the renamed original drop-in. This worked correctly in principle, however several popular plugins regularly check the file contents of the object-cache.php file to check whether their drop-in is present rather than checking for e.g. a constant that the file sets. Due to that behavior, in practice the approach the Performance Lab plugin has taken led to several incompatibilities. This PR fixes that problem going forward by removing that approach. Now the Performance Lab drop-in will only be placed if no conflicting drop-in is present.
  • Some code around the object-cache-plst-orig.php file is maintained for backward compatibility: For example, for sites for which that approach has been working as expected, it will need to be ensured that the deactivation routine not only deletes the Performance Lab object-cache.php drop-in, but also restores the original drop-in (i.e. renames object-cache-plst-orig.php back to object-cache.php).
  • Several comments have been added to explain the changes so that the context is clear for any future reference when looking at the code.
  • The PR also adds a function_exists() check around the one function that the Performance Lab drop-in defines to prevent any fatal error e.g. in case another drop-in like advanced-cache.php loads the object-cache.php drop-in extra early.

Testing instructions

Without existing drop-in

  1. Make sure there is no wp-content/object-cache.php file in your setup.
  2. Activate PL plugin in WP Admin.
  3. Validate that the wp-content/object-cache.php file now exists and has the same content as the server-timing/object-cache.copy.php file within the plugin.
  4. Deactivate the PL plugin.
  5. Validate that the wp-content/object-cache.php file no longer exists.

With drop-in already present

  1. Manually copy the plugin's server-timing/object-cache.copy.php file to wp-content/object-cache.php.
  2. Activate PL plugin in WP Admin.
  3. Validate that the wp-content/object-cache.php file still exists and has the same content as the server-timing/object-cache.copy.php file within the plugin.
  4. Deactivate the PL plugin.
  5. Validate that the wp-content/object-cache.php file no longer exists.

With conflicting drop-in present

  1. Place a custom PHP file (e.g. with just a comment, or you could use an actual object-cache.php drop-in from a plugin like e.g. W3 Total Cache) in wp-content/object-cache.php.
  2. Activate PL plugin in WP Admin.
  3. Validate that the wp-content/object-cache.php file still exists and has the same content as before. The plugin's own file should not replace it.
  4. Deactivate the PL plugin.
  5. Validate that the wp-content/object-cache.php file still exists (as it is not our own, so we shouldn't remove it).

With old code (our drop-in plus renamed other drop-in)

  1. Manually copy the plugin's server-timing/object-cache.copy.php file to wp-content/object-cache.php.
  2. Place a custom PHP file (e.g. with just a comment, or you could use an actual object-cache.php drop-in from a plugin like e.g. W3 Total Cache) in wp-content/object-cache-plst-orig.php.
  3. Activate PL plugin in WP Admin.
  4. Validate that the wp-content/object-cache.php and wp-content/object-cache-plst-orig.php files still exist and have the same content as before.
  5. Deactivate the PL plugin.
  6. Validate that the wp-content/object-cache.php file now has the content that previously was in the wp-content/object-cache-plst-orig.php file, and that the wp-content/object-cache-plst-orig.php file is now removed (as in that case we need to restore the other drop-in in its original location, which was something the plugin used to do, so this is critical for backward compatibility with older PL plugin versions).

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

Sorry, something went wrong.

@felixarntz felixarntz added [Type] Bug An existing feature is broken Infrastructure Issues for the overall performance plugin infrastructure labels Mar 14, 2023
@felixarntz felixarntz added this to the 2.1.0 milestone Mar 14, 2023
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @felixarntz, For each test case, PR functions properly.

@10upsimon
Copy link
Contributor

@felixarntz I've tested all scenarios in a wp-env and all passed. Approving.

@felixarntz felixarntz merged commit 3872aff into trunk Mar 15, 2023
@felixarntz felixarntz deleted the fix/630-drop-in-problems branch March 15, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Bug An existing feature is broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revisit how we handle object-cache.php conflicts
4 participants