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

Update SiteCapabilities to use Transient class #99

Merged
merged 14 commits into from
Sep 11, 2024

Conversation

BrianHenryIE
Copy link
Contributor

@BrianHenryIE BrianHenryIE commented Sep 10, 2024

Proposed changes

The Transient class was created to address an issue where sites have an object cache present which breaks transients on the site. E.g. when a site has migrated in and continues to try to use a Redis instance that does not exist. It works by detecting object-cache.php and using wp_options table for transients if that is present.

The SiteCapabilities class was not using this implementation.

This PR updates SiteCapabilities to use the Transient class, and adds tests for both classes.

There is a small break where currently the wp_options based transients are stored with an expires key which has been renamed to expires_at. This means the transients will return false the next time they are fetched and then once set again will work as expected.

Type of Change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Video

Checklist

  • I have read the CONTRIBUTING doc
  • I have viewed my change in a web-browser
  • Linting and tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

  • There are still other classes which are using get_transient() etc. rather than the Transient class
  • It may be possible to implement WP_Object_Cache as a global solution to the transient issue
  • I tried defining set_transient() etc in the NewfoldLabs\WP namespace hoping calls from inside NewfoldLabs\WP\Module\Data\* would use that rather than \set_transient() but it only works for the same namespace and skips functions defined in intermediate namespaces

*/
public static function should_use_transients() {
protected static function should_use_transients(): bool {
require_once constant( 'ABSPATH' ) . '/wp-admin/includes/plugin.php';
return ! array_key_exists( 'object-cache.php', get_dropins() );
Copy link
Member

Choose a reason for hiding this comment

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

@BrianHenryIE - We should probably update the logic here. This is true for shared hosting where there is no object cache, but now that we have Bluehost Cloud that does have object caching, we should take this into account.

Copy link
Contributor Author

@BrianHenryIE BrianHenryIE Sep 11, 2024

Choose a reason for hiding this comment

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

@BrianHenryIE BrianHenryIE merged commit 89f0957 into main Sep 11, 2024
6 of 7 checks passed
@BrianHenryIE BrianHenryIE deleted the Add/capabilities-tests branch September 13, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants