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

Add ability to prevent storing parsed CSS in transients #4177

Merged
merged 33 commits into from
Mar 23, 2020
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
ece53ed
Add filter to prevent storing parsed CSS in transients
westonruter Jan 25, 2020
217cc4b
Rename use_cache to use_object_cache
schlessera Mar 19, 2020
4d2bdba
Add infrastructure for managing background tasks
schlessera Mar 19, 2020
2accca5
Add logic to monitor CSS caching and disable if needed
schlessera Mar 19, 2020
a622435
Make use of AMP Options Manager and skip transients as needed
schlessera Mar 19, 2020
6600b16
Add tests
schlessera Mar 19, 2020
164dac2
Use assertInternalType instead of assertIsInt for PHPUnit back-compat
westonruter Mar 19, 2020
d28a5ef
Fix test_amp_get_content_sanitizers for added allow_transient_caching
westonruter Mar 19, 2020
2b08f22
Document filters
schlessera Mar 20, 2020
d7e506f
Refactor and simplify cron handling
schlessera Mar 20, 2020
ba094e1
Add srteamlined way of adding services, with activation and deactivation
schlessera Mar 20, 2020
61e358e
Add Site Health integration
schlessera Mar 20, 2020
10665d6
Turn maybe_instantiate() into instances() and return those
schlessera Mar 20, 2020
b34eadf
Short-circuit monitoring when an object cache is being used
schlessera Mar 20, 2020
a7646b2
Add todo for multisite blog iteration on (de)activation hooks
schlessera Mar 20, 2020
08506c2
Pass $network_wide through (de)activation chain
schlessera Mar 20, 2020
39e1d39
Iterate over sites for unscheduling on deactivation
schlessera Mar 20, 2020
8210c98
Schedule events on admin_init
schlessera Mar 20, 2020
d229af9
Use get_sites() instead of a direct SQL query
schlessera Mar 21, 2020
6542b7e
Check against a capability, not a role
schlessera Mar 21, 2020
eaf8259
Add warnings to network plugins screen
schlessera Mar 21, 2020
83d8a8d
Merge branch 'develop' into add/disabling-parsed-css-transient-caching
schlessera Mar 21, 2020
0362ea4
Fix Test_Monitor_CSS_Transient_Caching
westonruter Mar 21, 2020
2683dce
Merge branch 'develop' of github.com:ampproject/amp-wp into add/disab…
westonruter Mar 21, 2020
43affb0
Remove unused use statements
westonruter Mar 21, 2020
27b3039
Ensure warning icon uses color emoji and warning can be translated
westonruter Mar 21, 2020
7dd5c78
Split boolean state for transient caching to be tri-state indicating …
westonruter Mar 21, 2020
9ad9b10
Fix calling non-static method non-statically
westonruter Mar 22, 2020
a0995ff
Use non-static method invocations
westonruter Mar 22, 2020
7d3163f
Add linting to ensure amp.php is compatibility with PHP 5.2
westonruter Mar 22, 2020
686edc7
Make amp.php compatibility with PHP 5.2
westonruter Mar 22, 2020
80d1bf3
Revert using __DIR__ in bootstrap file
schlessera Mar 23, 2020
e53a3da
Remove unused constant
schlessera Mar 23, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
* @package AMP
*/

use AmpProject\AmpWP\Services;

define( 'AMP__FILE__', __FILE__ );
define( 'AMP__DIR__', __DIR__ );
define( 'AMP__VERSION', '1.5.0-alpha' );
Expand Down Expand Up @@ -268,14 +270,19 @@ function _amp_xdebug_admin_notice() {

require_once AMP__DIR__ . '/vendor/autoload.php';

Services::register();

register_activation_hook( __FILE__, 'amp_activate' );

/**
* Handle activation of plugin.
*
* @since 0.2
*
* @param bool $network_wide Whether the activation was done network-wide.
*/
function amp_activate() {
function amp_activate( $network_wide = false ) {
Services::activate( $network_wide );
amp_after_setup_theme();
if ( ! did_action( 'amp_init' ) ) {
amp_init();
Expand All @@ -289,8 +296,11 @@ function amp_activate() {
* Handle deactivation of plugin.
*
* @since 0.2
*
* @param bool $network_wide Whether the activation was done network-wide.
*/
function amp_deactivate() {
function amp_deactivate( $network_wide = false ) {
Services::deactivate( $network_wide );
// We need to manually remove the amp endpoint.
global $wp_rewrite;
foreach ( $wp_rewrite->endpoints as $index => $endpoint ) {
Expand Down
11 changes: 11 additions & 0 deletions includes/amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,17 @@ function amp_get_content_sanitizers( $post = null ) {
);
}

/**
* Filters whether parsed CSS is allowed to be cached in transients.
*
* When this is filtered to be false, parsed CSS will not be stored in transients. This is important when there is
* highly-variable CSS content in order to prevent filling up the wp_options table with an endless number of entries.
*
* @since 1.5.0
* @param bool $transient_caching_allowed Transient caching allowed.
*/
$sanitizers['AMP_Style_Sanitizer']['allow_transient_caching'] = apply_filters( 'amp_parsed_css_transient_caching_allowed', true );

// Force style sanitizer and whitelist sanitizer to be at end.
foreach ( [ 'AMP_Style_Sanitizer', 'AMP_Meta_Sanitizer', 'AMP_Tag_And_Attribute_Sanitizer' ] as $class_name ) {
if ( isset( $sanitizers[ $class_name ] ) ) {
Expand Down
46 changes: 36 additions & 10 deletions includes/sanitizers/class-amp-style-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* @package AMP
*/

use AmpProject\AmpWP\BackgroundTask\MonitorCssTransientCaching;
use AmpProject\AmpWP\Option;
use AmpProject\AmpWP\RemoteRequest\CachedRemoteGetRequest;
use AmpProject\AmpWP\RemoteRequest\WpHttpRemoteGetRequest;
use AmpProject\DevMode;
Expand Down Expand Up @@ -122,6 +124,7 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
* @type string $parsed_cache_variant Additional value by which to vary parsed cache.
* @type string[] $focus_within_classes Class names in selectors that should be replaced with :focus-within pseudo classes.
* @type string[] $low_priority_plugins Plugin slugs of the plugins to deprioritize when hitting the CSS limit.
* @type bool $allow_transient_caching Whether to allow caching parsed CSS in transients. This may need to be disabled when there is highly-variable CSS content.
* }
*/
protected $args;
Expand All @@ -143,6 +146,7 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
'parsed_cache_variant' => null,
'focus_within_classes' => [ 'focus' ],
'low_priority_plugins' => [ 'query-monitor' ],
'allow_transient_caching' => true,
];

/**
Expand Down Expand Up @@ -1484,10 +1488,11 @@ private function fetch_external_stylesheet( $url ) {
* }
*/
private function get_parsed_stylesheet( $stylesheet, $options = [] ) {
$parsed = null;
$cache_key = null;
$cached = true;
$cache_group = 'amp-parsed-stylesheet-v27'; // This should be bumped whenever the PHP-CSS-Parser is updated or parsed format is updated.
$parsed = null;
$cache_key = null;
$cached = true;
$cache_group = 'amp-parsed-stylesheet-v27'; // This should be bumped whenever the PHP-CSS-Parser is updated or parsed format is updated.
$use_transients = $this->should_use_transient_caching();

$cache_impacting_options = array_merge(
wp_array_slice_assoc(
Expand All @@ -1505,10 +1510,10 @@ private function get_parsed_stylesheet( $stylesheet, $options = [] ) {

$cache_key = md5( $stylesheet . wp_json_encode( $cache_impacting_options ) );

if ( wp_using_ext_object_cache() ) {
$parsed = wp_cache_get( $cache_key, $cache_group );
} else {
if ( $use_transients ) {
$parsed = get_transient( $cache_group . '-' . $cache_key );
} else {
$parsed = wp_cache_get( $cache_key, $cache_group );
}

/*
Expand All @@ -1534,18 +1539,39 @@ private function get_parsed_stylesheet( $stylesheet, $options = [] ) {
* getting filled infinitely. On the other hand, if an external object cache is available then we don't
* set an expiration because it should implement LRU cache expulsion policy.
*/
if ( wp_using_ext_object_cache() ) {
wp_cache_set( $cache_key, $parsed, $cache_group );
} else {
if ( $use_transients ) {
// The expiration is to ensure transient doesn't stick around forever since no LRU flushing like with external object cache.
set_transient( $cache_group . '-' . $cache_key, $parsed, MONTH_IN_SECONDS );
} else {
wp_cache_set( $cache_key, $parsed, $cache_group );
}
}

$parsed['cached'] = $cached;
return $parsed;
}

/**
* Check whether transient caching for stylesheets should be used.
*
* @return bool Whether transient caching should be used.
*/
private function should_use_transient_caching() {
if ( wp_using_ext_object_cache() ) {
return false;
}

if ( ! $this->args['allow_transient_caching'] ) {
return false;
}

if ( AMP_Options_Manager::get_option( Option::DISABLE_CSS_TRANSIENT_CACHING, false ) ) {
return false;
}

return true;
}

/**
* Parse imported stylesheet and replace the `@import` rule with the imported rules in the provided CSS list (in place).
*
Expand Down
Loading