diff --git a/.phpstorm.meta.php b/.phpstorm.meta.php index e59900f24df..804117ba0e1 100644 --- a/.phpstorm.meta.php +++ b/.phpstorm.meta.php @@ -33,6 +33,9 @@ 'server_timing' => \AmpProject\AmpWP\Instrumentation\ServerTiming::class, 'site_health_integration' => \AmpProject\AmpWP\Admin\SiteHealth::class, 'validated_url_stylesheet_gc' => \AmpProject\AmpWP\BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection::class, + 'url_validation_cron' => \AmpProject\AmpWP\Validation\URLValidationCron::class, + 'save_post_validation_event' => \AmpProject\AmpWP\Validation\SavePostValidationEvent::class, + 'background_task_deactivator' => \AmpProject\AmpWP\BackgroundTask\BackgroundTaskDeactivator::class, ] ) ); diff --git a/docs/class/README.md b/docs/class/README.md index 80c349178f6..a2bd7ed671e 100644 --- a/docs/class/README.md +++ b/docs/class/README.md @@ -2,5 +2,3 @@ * [`AMP_Base_Embed_Handler`](AMP_Base_Embed_Handler.md) - Class AMP_Base_Embed_Handler * [`AMP_Base_Sanitizer`](AMP_Base_Sanitizer.md) - Class AMP_Base_Sanitizer -* [`Validation\ScannableURLProvider`](Validation/ScannableURLProvider.md) - ScannableURLProvider class. -* [`Validation\URLValidationProvider`](Validation/URLValidationProvider.md) - URLValidationProvider class. diff --git a/docs/class/Validation/ScannableURLProvider.md b/docs/class/Validation/ScannableURLProvider.md deleted file mode 100644 index 0f780c24440..00000000000 --- a/docs/class/Validation/ScannableURLProvider.md +++ /dev/null @@ -1,310 +0,0 @@ -## Class `AmpProject\AmpWP\Validation\ScannableURLProvider` - -ScannableURLProvider class. - -### Methods - -* [`__construct`](../method/Validation/ScannableURLProvider/__construct.md) - Class constructor. -* [`get_urls`](../method/Validation/ScannableURLProvider/get_urls.md) - Provides the array of URLs to check. -### Source - -:link: [src/Validation/ScannableURLProvider.php:19](/src/Validation/ScannableURLProvider.php#L19-L309) - -
-Show Code - -```php -final class ScannableURLProvider { - - /** - * Whether to include URLs that don't support AMP. - * - * @var bool - */ - private $include_unsupported; - - /** - * An allowlist of conditionals to use for querying URLs. - * - * Usually, this class will query all of the templates that don't have AMP disabled. This allows inclusion based on only these conditionals. - * - * @var string[] - */ - private $include_conditionals; - - /** - * The maximum number of URLs to provide for each content type. - * - * Templates are each a separate type, like those for is_category() and is_tag(), and each post type is a type. - * - * @var int - */ - private $limit_per_type; - - /** - * Class constructor. - * - * @param integer $limit_per_type The maximum number of URLs to validate for each type. - * @param array $include_conditionals An allowlist of conditionals to use for validation. - * @param boolean $include_unsupported Whether to include URLs that don't support AMP. - */ - public function __construct( - $limit_per_type = 20, - $include_conditionals = [], - $include_unsupported = false - ) { - $this->limit_per_type = $limit_per_type; - $this->include_conditionals = $include_conditionals; - $this->include_unsupported = $include_unsupported; - } - - /** - * Provides the array of URLs to check. - * - * Each URL is an array with two elements, with the URL at index 0 and the type at index 1. - * - * @param int|null $offset Optional. The number of URLs to offset by, where applicable. Defaults to 0. - * @return array Array of URLs and types. - */ - public function get_urls( $offset = 0 ) { - $urls = []; - - /* - * If 'Your homepage displays' is set to 'Your latest posts', include the homepage. - */ - if ( 'posts' === get_option( 'show_on_front' ) && $this->is_template_supported( 'is_home' ) ) { - $urls[] = [ - 'url' => home_url( '/' ), - 'type' => 'home', - ]; - } - - $amp_enabled_taxonomies = array_filter( - get_taxonomies( [ 'public' => true ] ), - [ $this, 'does_taxonomy_support_amp' ] - ); - $public_post_types = get_post_types( [ 'public' => true ] ); - - // Include one URL of each template/content type, then another URL of each type on the next iteration. - for ( $i = $offset; $i < $this->limit_per_type + $offset; $i++ ) { - // Include all public, published posts. - foreach ( $public_post_types as $post_type ) { - $post_ids = $this->get_posts_that_support_amp( $this->get_posts_by_type( $post_type, $i, 1 ) ); - if ( ! empty( $post_ids[0] ) ) { - $urls[] = [ - 'url' => get_permalink( $post_ids[0] ), - 'type' => $post_type, - ]; - } - } - - foreach ( $amp_enabled_taxonomies as $taxonomy ) { - $taxonomy_links = $this->get_taxonomy_links( $taxonomy, $i, 1 ); - $link = reset( $taxonomy_links ); - if ( ! empty( $link ) ) { - $urls[] = [ - 'url' => $link, - 'type' => $taxonomy, - ]; - } - } - - $author_page_urls = $this->get_author_page_urls( $i, 1 ); - if ( ! empty( $author_page_urls[0] ) ) { - $urls[] = [ - 'url' => $author_page_urls[0], - 'type' => 'author', - ]; - } - } - - // Only validate 1 date and 1 search page. - $url = $this->get_date_page(); - if ( $url ) { - $urls[] = [ - 'url' => $url, - 'type' => 'date', - ]; - } - $url = $this->get_search_page(); - if ( $url ) { - $urls[] = [ - 'url' => $url, - 'type' => 'search', - ]; - } - - return $urls; - } - - /** - * Gets whether the template is supported. - * - * @param string $template The template to check. - * @return bool Whether the template is supported. - */ - private function is_template_supported( $template ) { - // If we received an allowlist of conditionals, this template conditional must be present in it. - if ( ! empty( $this->include_conditionals ) ) { - return in_array( $template, $this->include_conditionals, true ); - } - if ( $this->include_unsupported ) { - return true; - } - - $supportable_templates = AMP_Theme_Support::get_supportable_templates(); - - // Check whether this taxonomy's template is supported, including in the 'AMP Settings' > 'Supported Templates' UI. - return ! empty( $supportable_templates[ $template ]['supported'] ); - } - - /** - * Gets the post IDs that support AMP. - * - * By default, this only gets the post IDs if they support AMP. - * This means that 'Posts' isn't deselected in 'AMP Settings' > 'Supported Templates' - * and 'Enable AMP' isn't unchecked in the post's editor. - * - * @param int[] $ids The post IDs to check for AMP support. - * @return array The post IDs that support AMP, or an empty array. - */ - private function get_posts_that_support_amp( $ids ) { - if ( ! $this->is_template_supported( 'is_singular' ) ) { - return []; - } - - if ( $this->include_unsupported ) { - return $ids; - } - - return array_filter( - $ids, - 'post_supports_amp' - ); - } - - /** - * Gets the IDs of public, published posts. - * - * @param string $post_type The post type. - * @param int|null $offset The offset of the query (optional). - * @param int|null $number The number of posts to query for (optional). - * @return int[] $post_ids The post IDs in an array. - */ - private function get_posts_by_type( $post_type, $offset = null, $number = null ) { - $args = [ - 'post_type' => $post_type, - 'posts_per_page' => is_int( $number ) ? $number : $this->limit_per_type, - 'post_status' => 'publish', - 'orderby' => 'ID', - 'order' => 'DESC', - 'fields' => 'ids', - ]; - if ( is_int( $offset ) ) { - $args['offset'] = $offset; - } - - // Attachment posts usually have the post_status of 'inherit,' so they can use the status of the post they're attached to. - if ( 'attachment' === $post_type ) { - $args['post_status'] = 'inherit'; - } - $query = new WP_Query( $args ); - - return $query->posts; - } - - /** - * Gets the author page URLs, like https://example.com/author/admin/. - * - * Accepts an $offset parameter, for the query of authors. - * 0 is the first author in the query, and 1 is the second. - * - * @param int|string $offset The offset for the URL to query for, should be an int if passing an argument. - * @param int|string $number The total number to query for, should be an int if passing an argument. - * @return string[] The author page URLs, or an empty array. - */ - private function get_author_page_urls( $offset = '', $number = '' ) { - $author_page_urls = []; - if ( ! $this->is_template_supported( 'is_author' ) ) { - return $author_page_urls; - } - - $number = ! empty( $number ) ? $number : $this->limit_per_type; - foreach ( get_users( compact( 'offset', 'number' ) ) as $author ) { - $author_page_urls[] = get_author_posts_url( $author->ID, $author->user_nicename ); - } - - return $author_page_urls; - } - - /** - * Gets a single search page URL, like https://example.com/?s=example. - * - * @return string|null An example search page, or null. - */ - private function get_search_page() { - if ( ! $this->is_template_supported( 'is_search' ) ) { - return null; - } - - return add_query_arg( 's', 'example', home_url( '/' ) ); - } - - /** - * Gets a single date page URL, like https://example.com/?year=2018. - * - * @return string|null An example search page, or null. - */ - private function get_date_page() { - if ( ! $this->is_template_supported( 'is_date' ) ) { - return null; - } - - return add_query_arg( 'year', gmdate( 'Y' ), home_url( '/' ) ); - } - - /** - * Gets whether the taxonomy supports AMP. - * - * @param string $taxonomy The taxonomy. - * @return boolean Whether the taxonomy supports AMP. - */ - private function does_taxonomy_support_amp( $taxonomy ) { - if ( 'post_tag' === $taxonomy ) { - $taxonomy = 'tag'; - } - $taxonomy_key = 'is_' . $taxonomy; - $custom_taxonomy_key = sprintf( 'is_tax[%s]', $taxonomy ); - return $this->is_template_supported( $taxonomy_key ) || $this->is_template_supported( $custom_taxonomy_key ); - } - - /** - * Gets the front-end links for taxonomy terms. - * For example, https://example.org/?cat=2 - * - * @param string $taxonomy The name of the taxonomy, like 'category' or 'post_tag'. - * @param int|string $offset The number at which to offset the query (optional). - * @param int $number The maximum amount of links to get (optional). - * @return string[] The term links, as an array of strings. - */ - private function get_taxonomy_links( $taxonomy, $offset = '', $number = null ) { - if ( is_null( $number ) ) { - $number = $this->limit_per_type; - } - - return array_map( - 'get_term_link', - get_terms( - array_merge( - compact( 'taxonomy', 'offset', 'number' ), - [ - 'orderby' => 'id', - ] - ) - ) - ); - } -} -``` - -
diff --git a/docs/class/Validation/URLValidationProvider.md b/docs/class/Validation/URLValidationProvider.md deleted file mode 100644 index 2b2cf16d3de..00000000000 --- a/docs/class/Validation/URLValidationProvider.md +++ /dev/null @@ -1,248 +0,0 @@ -## Class `AmpProject\AmpWP\Validation\URLValidationProvider` - -URLValidationProvider class. - -### Methods - -* [`is_locked`](../method/Validation/URLValidationProvider/is_locked.md) - Returns whether validation is currently locked. -* [`with_lock`](../method/Validation/URLValidationProvider/with_lock.md) - Runs a callback with a lock set for the duration of the callback. -* [`reset_lock`](../method/Validation/URLValidationProvider/reset_lock.md) - Resets the lock timeout. This allows long-running processes to keep running beyond the lock timeout. -* [`get_total_errors`](../method/Validation/URLValidationProvider/get_total_errors.md) - Provides the total number of validation errors found. -* [`get_unaccepted_errors`](../method/Validation/URLValidationProvider/get_unaccepted_errors.md) - Provides the total number of unaccepted errors. -* [`get_number_validated`](../method/Validation/URLValidationProvider/get_number_validated.md) - Provides the number of URLs that have been checked. -* [`get_validity_by_type`](../method/Validation/URLValidationProvider/get_validity_by_type.md) - Provides the validity counts by type. -* [`get_url_validation`](../method/Validation/URLValidationProvider/get_url_validation.md) - Validates a URL, stores the results, and increments the counts. -### Source - -:link: [src/Validation/URLValidationProvider.php:21](/src/Validation/URLValidationProvider.php#L21-L243) - -
-Show Code - -```php -final class URLValidationProvider { - - /** - * Key for the transient signaling validation is locked. - * - * @var string - */ - const LOCK_KEY = 'amp_validation_locked'; - - /** - * The length of time to keep the lock in place if a process fails to unlock. - * - * @var int - */ - const LOCK_TIMEOUT = 5 * MINUTE_IN_SECONDS; - - /** - * The total number of validation errors, regardless of whether they were accepted. - * - * @var int - */ - private $total_errors = 0; - - /** - * The total number of unaccepted validation errors. - * - * If an error has been accepted in the /wp-admin validation UI, - * it won't count toward this. - * - * @var int - */ - private $unaccepted_errors = 0; - - /** - * The number of URLs crawled, regardless of whether they have validation errors. - * - * @var int - */ - private $number_validated = 0; - - /** - * The validation counts by type, like template or post type. - * - * @var array[] { - * Validity by type. - * - * @type array $type { - * @type int $valid The number of valid URLs for this type. - * @type int $total The total number of URLs for this type, valid or invalid. - * } - * } - */ - private $validity_by_type = []; - - /** - * Locks validation. - */ - private function lock() { - update_option( self::LOCK_KEY, time(), false ); - } - - /** - * Unlocks validation. - */ - private function unlock() { - delete_option( self::LOCK_KEY ); - } - - /** - * Returns whether validation is currently locked. - * - * @return boolean - */ - public function is_locked() { - $lock_time = (int) get_option( self::LOCK_KEY, 0 ); - - // It's locked if the difference between the lock time and the current time is less than the lockout time. - return time() - $lock_time < self::LOCK_TIMEOUT; - } - - /** - * Runs a callback with a lock set for the duration of the callback. - * - * @param callable $callback Callback to run with the lock set. - * @return mixed WP_Error if a lock is in place. Otherwise, the result of the callback or void if it doesn't return anything. - */ - public function with_lock( $callback ) { - if ( $this->is_locked() ) { - return new WP_Error( - 'amp_url_validation_locked', - __( 'URL validation cannot start right now because another process is already validating URLs. Try again in a few minutes.', 'amp' ) - ); - } - - $this->lock(); - $result = $callback(); - $this->unlock(); - - return $result; - } - - /** - * Resets the lock timeout. This allows long-running processes to keep running beyond the lock timeout. - */ - public function reset_lock() { - $this->lock(); - } - - /** - * Provides the total number of validation errors found. - * - * @return int - */ - public function get_total_errors() { - return $this->total_errors; - } - - /** - * Provides the total number of unaccepted errors. - * - * @return int - */ - public function get_unaccepted_errors() { - return $this->unaccepted_errors; - } - - /** - * Provides the number of URLs that have been checked. - * - * @return int - */ - public function get_number_validated() { - return $this->number_validated; - } - - /** - * Provides the validity counts by type. - * - * @return array[] - */ - public function get_validity_by_type() { - return $this->validity_by_type; - } - - /** - * Validates a URL, stores the results, and increments the counts. - * - * @param string $url The URL to validate. - * @param string $type The type of template, post, or taxonomy. - * @param bool $force_revalidate Whether to force revalidation regardless of whether the current results are stale. - * @return array|WP_Error Associative array containing validity result and whether the URL was revalidated, or a WP_Error on failure. - */ - public function get_url_validation( $url, $type, $force_revalidate = false ) { - $validity = null; - $revalidated = true; - - if ( ! $force_revalidate ) { - $url_post = AMP_Validated_URL_Post_Type::get_invalid_url_post( $url ); - - if ( $url_post && empty( AMP_Validated_URL_Post_Type::get_post_staleness( $url_post ) ) ) { - $validity = AMP_Validated_URL_Post_Type::get_invalid_url_validation_errors( $url_post ); - $revalidated = false; - } - } - - if ( is_null( $validity ) ) { - $validity = AMP_Validation_Manager::validate_url_and_store( $url ); - } - - if ( is_wp_error( $validity ) ) { - return $validity; - } - - if ( $validity && isset( $validity['results'] ) ) { - $this->update_state_from_validity( $validity, $type ); - } - - return compact( 'validity', 'revalidated' ); - } - - /** - * Increments crawl counts from a validation result. - * - * @param array $validity Validity results. - * @param string $type The URL type. - */ - private function update_state_from_validity( $validity, $type ) { - $validation_errors = wp_list_pluck( $validity['results'], 'error' ); - $unaccepted_error_count = count( - array_filter( - $validation_errors, - static function( $error ) { - $validation_status = AMP_Validation_Error_Taxonomy::get_validation_error_sanitization( $error ); - return ( - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_ACCEPTED_STATUS !== $validation_status['term_status'] - && - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_ACCEPTED_STATUS !== $validation_status['term_status'] - ); - } - ) - ); - - if ( count( $validation_errors ) > 0 ) { - $this->total_errors++; - } - if ( $unaccepted_error_count > 0 ) { - $this->unaccepted_errors++; - } - - $this->number_validated++; - - if ( ! isset( $this->validity_by_type[ $type ] ) ) { - $this->validity_by_type[ $type ] = [ - 'valid' => 0, - 'total' => 0, - ]; - } - $this->validity_by_type[ $type ]['total']++; - if ( 0 === $unaccepted_error_count ) { - $this->validity_by_type[ $type ]['valid']++; - } - } -} -``` - -
diff --git a/docs/method/README.md b/docs/method/README.md index 2b44ccad09c..93e0a275784 100644 --- a/docs/method/README.md +++ b/docs/method/README.md @@ -34,13 +34,3 @@ * [`AMP_Base_Sanitizer::sanitize_dimension()`](AMP_Base_Sanitizer/sanitize_dimension.md) - Sanitizes a CSS dimension specifier while being sensitive to dimension context. * [`AMP_Base_Sanitizer::set_layout()`](AMP_Base_Sanitizer/set_layout.md) - Sets the layout, and possibly the 'height' and 'width' attributes. * [`AMP_Base_Sanitizer::should_sanitize_validation_error()`](AMP_Base_Sanitizer/should_sanitize_validation_error.md) - Check whether or not sanitization should occur in response to validation error. -* [`ScannableURLProvider::__construct()`](Validation/ScannableURLProvider/__construct.md) - Class constructor. -* [`ScannableURLProvider::get_urls()`](Validation/ScannableURLProvider/get_urls.md) - Provides the array of URLs to check. -* [`URLValidationProvider::get_number_validated()`](Validation/URLValidationProvider/get_number_validated.md) - Provides the number of URLs that have been checked. -* [`URLValidationProvider::get_total_errors()`](Validation/URLValidationProvider/get_total_errors.md) - Provides the total number of validation errors found. -* [`URLValidationProvider::get_unaccepted_errors()`](Validation/URLValidationProvider/get_unaccepted_errors.md) - Provides the total number of unaccepted errors. -* [`URLValidationProvider::get_url_validation()`](Validation/URLValidationProvider/get_url_validation.md) - Validates a URL, stores the results, and increments the counts. -* [`URLValidationProvider::get_validity_by_type()`](Validation/URLValidationProvider/get_validity_by_type.md) - Provides the validity counts by type. -* [`URLValidationProvider::is_locked()`](Validation/URLValidationProvider/is_locked.md) - Returns whether validation is currently locked. -* [`URLValidationProvider::reset_lock()`](Validation/URLValidationProvider/reset_lock.md) - Resets the lock timeout. This allows long-running processes to keep running beyond the lock timeout. -* [`URLValidationProvider::with_lock()`](Validation/URLValidationProvider/with_lock.md) - Runs a callback with a lock set for the duration of the callback. diff --git a/docs/method/Validation/ScannableURLProvider/__construct.md b/docs/method/Validation/ScannableURLProvider/__construct.md deleted file mode 100644 index 6b9e72174f4..00000000000 --- a/docs/method/Validation/ScannableURLProvider/__construct.md +++ /dev/null @@ -1,34 +0,0 @@ -## Method `ScannableURLProvider::__construct()` - -```php -public function __construct( $limit_per_type = 20, $include_conditionals = array(), $include_unsupported = false ); -``` - -Class constructor. - -### Arguments - -* `integer $limit_per_type` - The maximum number of URLs to validate for each type. -* `array $include_conditionals` - An allowlist of conditionals to use for validation. -* `boolean $include_unsupported` - Whether to include URLs that don't support AMP. - -### Source - -:link: [src/Validation/ScannableURLProvider.php:53](/src/Validation/ScannableURLProvider.php#L53-L61) - -
-Show Code - -```php -public function __construct( - $limit_per_type = 20, - $include_conditionals = [], - $include_unsupported = false -) { - $this->limit_per_type = $limit_per_type; - $this->include_conditionals = $include_conditionals; - $this->include_unsupported = $include_unsupported; -} -``` - -
diff --git a/docs/method/Validation/ScannableURLProvider/get_urls.md b/docs/method/Validation/ScannableURLProvider/get_urls.md deleted file mode 100644 index 48c9491fff3..00000000000 --- a/docs/method/Validation/ScannableURLProvider/get_urls.md +++ /dev/null @@ -1,92 +0,0 @@ -## Method `ScannableURLProvider::get_urls()` - -```php -public function get_urls( $offset ); -``` - -Provides the array of URLs to check. - -Each URL is an array with two elements, with the URL at index 0 and the type at index 1. - -### Arguments - -* `int|null $offset` - Optional. The number of URLs to offset by, where applicable. Defaults to 0. - -### Return value - -`array` - Array of URLs and types. - -### Source - -:link: [src/Validation/ScannableURLProvider.php:71](/src/Validation/ScannableURLProvider.php#L71-L140) - -
-Show Code - -```php -public function get_urls( $offset = 0 ) { - $urls = []; - /* - * If 'Your homepage displays' is set to 'Your latest posts', include the homepage. - */ - if ( 'posts' === get_option( 'show_on_front' ) && $this->is_template_supported( 'is_home' ) ) { - $urls[] = [ - 'url' => home_url( '/' ), - 'type' => 'home', - ]; - } - $amp_enabled_taxonomies = array_filter( - get_taxonomies( [ 'public' => true ] ), - [ $this, 'does_taxonomy_support_amp' ] - ); - $public_post_types = get_post_types( [ 'public' => true ] ); - // Include one URL of each template/content type, then another URL of each type on the next iteration. - for ( $i = $offset; $i < $this->limit_per_type + $offset; $i++ ) { - // Include all public, published posts. - foreach ( $public_post_types as $post_type ) { - $post_ids = $this->get_posts_that_support_amp( $this->get_posts_by_type( $post_type, $i, 1 ) ); - if ( ! empty( $post_ids[0] ) ) { - $urls[] = [ - 'url' => get_permalink( $post_ids[0] ), - 'type' => $post_type, - ]; - } - } - foreach ( $amp_enabled_taxonomies as $taxonomy ) { - $taxonomy_links = $this->get_taxonomy_links( $taxonomy, $i, 1 ); - $link = reset( $taxonomy_links ); - if ( ! empty( $link ) ) { - $urls[] = [ - 'url' => $link, - 'type' => $taxonomy, - ]; - } - } - $author_page_urls = $this->get_author_page_urls( $i, 1 ); - if ( ! empty( $author_page_urls[0] ) ) { - $urls[] = [ - 'url' => $author_page_urls[0], - 'type' => 'author', - ]; - } - } - // Only validate 1 date and 1 search page. - $url = $this->get_date_page(); - if ( $url ) { - $urls[] = [ - 'url' => $url, - 'type' => 'date', - ]; - } - $url = $this->get_search_page(); - if ( $url ) { - $urls[] = [ - 'url' => $url, - 'type' => 'search', - ]; - } - return $urls; -} -``` - -
diff --git a/docs/method/Validation/URLValidationProvider/get_number_validated.md b/docs/method/Validation/URLValidationProvider/get_number_validated.md deleted file mode 100644 index 27b01eecf96..00000000000 --- a/docs/method/Validation/URLValidationProvider/get_number_validated.md +++ /dev/null @@ -1,26 +0,0 @@ -## Method `URLValidationProvider::get_number_validated()` - -```php -public function get_number_validated(); -``` - -Provides the number of URLs that have been checked. - -### Return value - -`int` - -### Source - -:link: [src/Validation/URLValidationProvider.php:152](/src/Validation/URLValidationProvider.php#L152-L154) - -
-Show Code - -```php -public function get_number_validated() { - return $this->number_validated; -} -``` - -
diff --git a/docs/method/Validation/URLValidationProvider/get_total_errors.md b/docs/method/Validation/URLValidationProvider/get_total_errors.md deleted file mode 100644 index 013fecab5dc..00000000000 --- a/docs/method/Validation/URLValidationProvider/get_total_errors.md +++ /dev/null @@ -1,26 +0,0 @@ -## Method `URLValidationProvider::get_total_errors()` - -```php -public function get_total_errors(); -``` - -Provides the total number of validation errors found. - -### Return value - -`int` - -### Source - -:link: [src/Validation/URLValidationProvider.php:134](/src/Validation/URLValidationProvider.php#L134-L136) - -
-Show Code - -```php -public function get_total_errors() { - return $this->total_errors; -} -``` - -
diff --git a/docs/method/Validation/URLValidationProvider/get_unaccepted_errors.md b/docs/method/Validation/URLValidationProvider/get_unaccepted_errors.md deleted file mode 100644 index 4272903890a..00000000000 --- a/docs/method/Validation/URLValidationProvider/get_unaccepted_errors.md +++ /dev/null @@ -1,26 +0,0 @@ -## Method `URLValidationProvider::get_unaccepted_errors()` - -```php -public function get_unaccepted_errors(); -``` - -Provides the total number of unaccepted errors. - -### Return value - -`int` - -### Source - -:link: [src/Validation/URLValidationProvider.php:143](/src/Validation/URLValidationProvider.php#L143-L145) - -
-Show Code - -```php -public function get_unaccepted_errors() { - return $this->unaccepted_errors; -} -``` - -
diff --git a/docs/method/Validation/URLValidationProvider/get_url_validation.md b/docs/method/Validation/URLValidationProvider/get_url_validation.md deleted file mode 100644 index 5fc7f618b02..00000000000 --- a/docs/method/Validation/URLValidationProvider/get_url_validation.md +++ /dev/null @@ -1,50 +0,0 @@ -## Method `URLValidationProvider::get_url_validation()` - -```php -public function get_url_validation( $url, $type, $force_revalidate = false ); -``` - -Validates a URL, stores the results, and increments the counts. - -### Arguments - -* `string $url` - The URL to validate. -* `string $type` - The type of template, post, or taxonomy. -* `bool $force_revalidate` - Whether to force revalidation regardless of whether the current results are stale. - -### Return value - -`array|\WP_Error` - Associative array containing validity result and whether the URL was revalidated, or a WP_Error on failure. - -### Source - -:link: [src/Validation/URLValidationProvider.php:173](/src/Validation/URLValidationProvider.php#L173-L199) - -
-Show Code - -```php -public function get_url_validation( $url, $type, $force_revalidate = false ) { - $validity = null; - $revalidated = true; - if ( ! $force_revalidate ) { - $url_post = AMP_Validated_URL_Post_Type::get_invalid_url_post( $url ); - if ( $url_post && empty( AMP_Validated_URL_Post_Type::get_post_staleness( $url_post ) ) ) { - $validity = AMP_Validated_URL_Post_Type::get_invalid_url_validation_errors( $url_post ); - $revalidated = false; - } - } - if ( is_null( $validity ) ) { - $validity = AMP_Validation_Manager::validate_url_and_store( $url ); - } - if ( is_wp_error( $validity ) ) { - return $validity; - } - if ( $validity && isset( $validity['results'] ) ) { - $this->update_state_from_validity( $validity, $type ); - } - return compact( 'validity', 'revalidated' ); -} -``` - -
diff --git a/docs/method/Validation/URLValidationProvider/get_validity_by_type.md b/docs/method/Validation/URLValidationProvider/get_validity_by_type.md deleted file mode 100644 index c8de8b90b1a..00000000000 --- a/docs/method/Validation/URLValidationProvider/get_validity_by_type.md +++ /dev/null @@ -1,26 +0,0 @@ -## Method `URLValidationProvider::get_validity_by_type()` - -```php -public function get_validity_by_type(); -``` - -Provides the validity counts by type. - -### Return value - -`array[]` - -### Source - -:link: [src/Validation/URLValidationProvider.php:161](/src/Validation/URLValidationProvider.php#L161-L163) - -
-Show Code - -```php -public function get_validity_by_type() { - return $this->validity_by_type; -} -``` - -
diff --git a/docs/method/Validation/URLValidationProvider/is_locked.md b/docs/method/Validation/URLValidationProvider/is_locked.md deleted file mode 100644 index b42e42e2f98..00000000000 --- a/docs/method/Validation/URLValidationProvider/is_locked.md +++ /dev/null @@ -1,28 +0,0 @@ -## Method `URLValidationProvider::is_locked()` - -```php -public function is_locked(); -``` - -Returns whether validation is currently locked. - -### Return value - -`boolean` - -### Source - -:link: [src/Validation/URLValidationProvider.php:94](/src/Validation/URLValidationProvider.php#L94-L99) - -
-Show Code - -```php -public function is_locked() { - $lock_time = (int) get_option( self::LOCK_KEY, 0 ); - // It's locked if the difference between the lock time and the current time is less than the lockout time. - return time() - $lock_time < self::LOCK_TIMEOUT; -} -``` - -
diff --git a/docs/method/Validation/URLValidationProvider/reset_lock.md b/docs/method/Validation/URLValidationProvider/reset_lock.md deleted file mode 100644 index 435504ddf00..00000000000 --- a/docs/method/Validation/URLValidationProvider/reset_lock.md +++ /dev/null @@ -1,22 +0,0 @@ -## Method `URLValidationProvider::reset_lock()` - -```php -public function reset_lock(); -``` - -Resets the lock timeout. This allows long-running processes to keep running beyond the lock timeout. - -### Source - -:link: [src/Validation/URLValidationProvider.php:125](/src/Validation/URLValidationProvider.php#L125-L127) - -
-Show Code - -```php -public function reset_lock() { - $this->lock(); -} -``` - -
diff --git a/docs/method/Validation/URLValidationProvider/with_lock.md b/docs/method/Validation/URLValidationProvider/with_lock.md deleted file mode 100644 index 765543679b3..00000000000 --- a/docs/method/Validation/URLValidationProvider/with_lock.md +++ /dev/null @@ -1,39 +0,0 @@ -## Method `URLValidationProvider::with_lock()` - -```php -public function with_lock( $callback ); -``` - -Runs a callback with a lock set for the duration of the callback. - -### Arguments - -* `callable $callback` - Callback to run with the lock set. - -### Return value - -`mixed` - WP_Error if a lock is in place. Otherwise, the result of the callback or void if it doesn't return anything. - -### Source - -:link: [src/Validation/URLValidationProvider.php:107](/src/Validation/URLValidationProvider.php#L107-L120) - -
-Show Code - -```php -public function with_lock( $callback ) { - if ( $this->is_locked() ) { - return new WP_Error( - 'amp_url_validation_locked', - __( 'URL validation cannot start right now because another process is already validating URLs. Try again in a few minutes.', 'amp' ) - ); - } - $this->lock(); - $result = $callback(); - $this->unlock(); - return $result; -} -``` - -
diff --git a/includes/cli/class-amp-cli-validation-command.php b/includes/cli/class-amp-cli-validation-command.php index 9a427c08426..e11268a898b 100644 --- a/includes/cli/class-amp-cli-validation-command.php +++ b/includes/cli/class-amp-cli-validation-command.php @@ -11,6 +11,7 @@ use AmpProject\AmpWP\Option; use AmpProject\AmpWP\Validation\URLValidationProvider; use AmpProject\AmpWP\Validation\ScannableURLProvider; +use AmpProject\AmpWP\Validation\URLScanningContext; use WP_CLI\Utils; /** @@ -141,16 +142,7 @@ public function run( /** @noinspection PhpUnusedParameterInspection */ $args, $a $number_urls_to_crawl ); - $result = $url_validation_provider->with_lock( - function () use ( $urls ) { - $this->validate_urls( $urls ); - } - ); - - if ( is_wp_error( $result ) ) { - WP_CLI::error( 'The site cannot be crawled at this time because validation is running in another process.' ); - return; - } + $this->validate_urls( $urls ); $this->wp_cli_progress->finish(); @@ -248,9 +240,11 @@ private function get_validation_url_provider() { } $this->scannable_url_provider = new ScannableURLProvider( - $limit_type_validate_count, - $include_conditionals, - $force_crawl_urls + new URLScanningContext( + $limit_type_validate_count, + $include_conditionals, + $force_crawl_urls + ) ); return $this->scannable_url_provider; @@ -284,12 +278,7 @@ private function validate_urls( $urls = null ) { $urls = $scannable_url_provider->get_urls(); } - foreach ( $urls as $index => $url ) { - // Reset lock between every five URLs. - if ( 0 === $index % 5 ) { - $this->url_validation_provider->reset_lock(); - } - + foreach ( $urls as $url ) { $validity = $url_validation_provider->get_url_validation( $url['url'], $url['type'], true ); if ( $this->wp_cli_progress ) { diff --git a/src/AmpWpPlugin.php b/src/AmpWpPlugin.php index 18ae82e38d1..88ac36848e7 100644 --- a/src/AmpWpPlugin.php +++ b/src/AmpWpPlugin.php @@ -11,6 +11,9 @@ use AmpProject\AmpWP\BackgroundTask; use AmpProject\AmpWP\Infrastructure\ServiceBasedPlugin; use AmpProject\AmpWP\Instrumentation; +use AmpProject\AmpWP\Validation\SavePostValidationEvent; +use AmpProject\AmpWP\Validation\URLValidationCron; +use AmpProject\AmpWP\BackgroundTask\BackgroundTaskDeactivator; use function is_user_logged_in; @@ -80,6 +83,9 @@ final class AmpWpPlugin extends ServiceBasedPlugin { 'server_timing' => Instrumentation\ServerTiming::class, 'site_health_integration' => Admin\SiteHealth::class, 'validated_url_stylesheet_gc' => BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection::class, + 'url_validation_cron' => URLValidationCron::class, + 'save_post_validation_event' => SavePostValidationEvent::class, + 'background_task_deactivator' => BackgroundTaskDeactivator::class, ]; /** @@ -161,6 +167,7 @@ protected function get_shared_instances() { DevTools\CallbackReflection::class, DevTools\FileReflection::class, ReaderThemeLoader::class, + BackgroundTask\BackgroundTaskDeactivator::class, ]; } diff --git a/src/BackgroundTask/BackgroundTaskDeactivator.php b/src/BackgroundTask/BackgroundTaskDeactivator.php new file mode 100644 index 00000000000..ea19fc60343 --- /dev/null +++ b/src/BackgroundTask/BackgroundTaskDeactivator.php @@ -0,0 +1,161 @@ +plugin_file = plugin_basename( AMP__FILE__ ); + } + + /** + * Register the service with the system. + * + * @return void + */ + public function register() { + add_action( "network_admin_plugin_action_links_{$this->plugin_file}", [ $this, 'add_warning_sign_to_network_deactivate_action' ], 10, 1 ); + add_action( 'plugin_row_meta', [ $this, 'add_warning_to_plugin_meta' ], 10, 2 ); + } + + /** + * Get warning icon markup. + * + * @return string Warning icon markup. + */ + private function get_warning_icon() { + return sprintf( '%s', Icon::warning()->to_html() ); + } + + /** + * Adds an event to the deactivate queue. + * + * @param string $event_name The event name. + */ + public function add_event( $event_name ) { + if ( ! in_array( $event_name, $this->events_to_deactivate, true ) ) { + $this->events_to_deactivate[] = $event_name; + } + } + + /** + * Run deactivation logic. + * + * This should be hooked up to the WordPress deactivation hook. + * + * @param bool $network_wide Whether the deactivation was done network-wide. + * @return void + */ + public function deactivate( $network_wide ) { + if ( $network_wide && is_multisite() && ! wp_is_large_network( 'sites' ) ) { + foreach ( get_sites( + [ + 'fields' => 'ids', + 'number' => 0, // Disables pagination to retrieve all sites. + ] + ) as $blog_id ) { + switch_to_blog( $blog_id ); + + foreach ( $this->events_to_deactivate as $event_name ) { + wp_unschedule_hook( $event_name ); + } + + restore_current_blog(); + } + } else { + foreach ( $this->events_to_deactivate as $event_name ) { + wp_unschedule_hook( $event_name ); + } + } + } + + /** + * Add a warning sign to the network deactivate action on the network plugins screen. + * + * @param string[] $actions An array of plugin action links. By default this can include 'activate', + * 'deactivate', and 'delete'. + * @return string[] + */ + public function add_warning_sign_to_network_deactivate_action( $actions ) { + if ( ! wp_is_large_network() ) { + return $actions; + } + + if ( ! array_key_exists( 'deactivate', $actions ) ) { + return $actions; + } + + wp_enqueue_style( 'amp-icons' ); + + $warning_icon = $this->get_warning_icon(); + if ( false === strpos( $actions['deactivate'], $warning_icon ) ) { + $actions['deactivate'] = preg_replace( '#(?=)#i', ' ' . $warning_icon, $actions['deactivate'] ); + } + + return $actions; + } + + /** + * Add a warning to the plugin meta row on the network plugins screen. + * + * @param string[] $plugin_meta An array of the plugin's metadata, including the version, author, author URI, and + * plugin URI. + * @param string $plugin_file Path to the plugin file relative to the plugins directory. + * @return string[] + */ + public function add_warning_to_plugin_meta( $plugin_meta, $plugin_file ) { + if ( ! is_multisite() || ! wp_is_large_network() ) { + return $plugin_meta; + } + + if ( $plugin_file !== $this->plugin_file ) { + return $plugin_meta; + } + + wp_enqueue_style( 'amp-icons' ); + + $warning = $this->get_warning_icon() . ' ' . esc_html__( 'Large site detected. Deactivation will leave orphaned scheduled events behind.', 'amp' ) . ' ' . $this->get_warning_icon(); + + if ( ! in_array( $warning, $plugin_meta, true ) ) { + $plugin_meta[] = $warning; + } + + return $plugin_meta; + } +} diff --git a/src/BackgroundTask/CronBasedBackgroundTask.php b/src/BackgroundTask/CronBasedBackgroundTask.php index aae61ea5c04..925e81ed4a6 100644 --- a/src/BackgroundTask/CronBasedBackgroundTask.php +++ b/src/BackgroundTask/CronBasedBackgroundTask.php @@ -7,9 +7,6 @@ namespace AmpProject\AmpWP\BackgroundTask; -use AmpProject\AmpWP\Icon; -use AmpProject\AmpWP\Infrastructure\Conditional; -use AmpProject\AmpWP\Infrastructure\Deactivateable; use AmpProject\AmpWP\Infrastructure\Registerable; use AmpProject\AmpWP\Infrastructure\Service; @@ -20,28 +17,26 @@ * @since 2.0 * @internal */ -abstract class CronBasedBackgroundTask implements Service, Registerable, Conditional, Deactivateable { +abstract class CronBasedBackgroundTask implements Service, Registerable { const DEFAULT_INTERVAL_HOURLY = 'hourly'; const DEFAULT_INTERVAL_TWICE_DAILY = 'twicedaily'; const DEFAULT_INTERVAL_DAILY = 'daily'; /** - * Name of the plugin as WordPress is expecting it. + * BackgroundTaskDeactivator instance. * - * This should usually have the form "amp/amp.php". - * - * @var string + * @var BackgroundTaskDeactivator */ - private $plugin_file; + protected $background_task_deactivator; /** - * Check whether the conditional object is currently needed. + * Class constructor. * - * @return bool Whether the conditional object is needed. + * @param BackgroundTaskDeactivator $background_task_deactivator Service that deactivates background events. */ - public static function is_needed() { - return is_admin() || wp_doing_cron(); + public function __construct( BackgroundTaskDeactivator $background_task_deactivator ) { + $this->background_task_deactivator = $background_task_deactivator; } /** @@ -50,132 +45,15 @@ public static function is_needed() { * @return void */ public function register() { - add_action( 'admin_init', [ $this, 'schedule_event' ] ); - add_action( $this->get_event_name(), [ $this, 'process' ] ); - - $this->plugin_file = plugin_basename( dirname( dirname( __DIR__ ) ) . '/amp.php' ); - add_action( "network_admin_plugin_action_links_{$this->plugin_file}", [ $this, 'add_warning_sign_to_network_deactivate_action' ], 10, 1 ); - add_action( 'plugin_row_meta', [ $this, 'add_warning_to_plugin_meta' ], 10, 2 ); - } - - /** - * Get warning icon markup. - * - * @return string Warning icon markup. - */ - private function get_warning_icon() { - return sprintf( '%s', Icon::warning()->to_html() ); + $this->background_task_deactivator->add_event( $this->get_event_name() ); } /** * Schedule the event. * - * This does nothing if the event is already scheduled. - * - * @return void + * @param mixed[] ...$args Arguments passed to the function from the action hook. */ - public function schedule_event() { - if ( ! current_user_can( 'manage_options' ) ) { - return; - } - - $event_name = $this->get_event_name(); - $timestamp = wp_next_scheduled( $event_name ); - - if ( $timestamp ) { - return; - } - - wp_schedule_event( time(), $this->get_interval(), $event_name ); - } - - /** - * Run deactivation logic. - * - * This should be hooked up to the WordPress deactivation hook. - * - * @todo Needs refactoring if used for more than one cron-based task, to avoid iterating over sites multiple times. - * - * @param bool $network_wide Whether the deactivation was done network-wide. - * @return void - */ - public function deactivate( $network_wide ) { - if ( $network_wide && is_multisite() && ! wp_is_large_network( 'sites' ) ) { - foreach ( get_sites( - [ - 'fields' => 'ids', - 'number' => 0, - ] - ) as $blog_id ) { - switch_to_blog( $blog_id ); - wp_clear_scheduled_hook( $this->get_event_name() ); - restore_current_blog(); - } - } else { - wp_clear_scheduled_hook( $this->get_event_name() ); - } - } - - /** - * Add a warning sign to the network deactivate action on the network plugins screen. - * - * @param string[] $actions An array of plugin action links. By default this can include 'activate', - * 'deactivate', and 'delete'. - * @return string[] - */ - public function add_warning_sign_to_network_deactivate_action( $actions ) { - if ( ! wp_is_large_network() ) { - return $actions; - } - - if ( ! array_key_exists( 'deactivate', $actions ) ) { - return $actions; - } - - wp_enqueue_style( 'amp-icons' ); - - $warning_icon = $this->get_warning_icon(); - if ( false === strpos( $actions['deactivate'], $warning_icon ) ) { - $actions['deactivate'] = preg_replace( '#(?=)#i', ' ' . $warning_icon, $actions['deactivate'] ); - } - - return $actions; - } - - /** - * Add a warning to the plugin meta row on the network plugins screen. - * - * @param string[] $plugin_meta An array of the plugin's metadata, including the version, author, author URI, and - * plugin URI. - * @param string $plugin_file Path to the plugin file relative to the plugins directory. - * @return string[] - */ - public function add_warning_to_plugin_meta( $plugin_meta, $plugin_file ) { - if ( ! is_multisite() || ! wp_is_large_network() ) { - return $plugin_meta; - } - - if ( $plugin_file !== $this->plugin_file ) { - return $plugin_meta; - } - - wp_enqueue_style( 'amp-icons' ); - - $warning = $this->get_warning_icon() . ' ' . esc_html__( 'Large site detected. Deactivation will leave orphaned scheduled events behind.', 'amp' ) . ' ' . $this->get_warning_icon(); - - if ( ! in_array( $warning, $plugin_meta, true ) ) { - $plugin_meta[] = $warning; - } - - return $plugin_meta; - } - - /** - * Get the interval to use for the event. - * - * @return string An existing interval name. Valid values are 'hourly', 'twicedaily' or 'daily'. - */ - abstract protected function get_interval(); + abstract protected function schedule_event( ...$args ); /** * Get the event name. @@ -189,9 +67,9 @@ abstract protected function get_interval(); abstract protected function get_event_name(); /** - * Process a single cron tick. + * Process the event. * - * @return void + * @param mixed[] ...$args Args to pass to the process callback. */ - abstract public function process(); + abstract public function process( ...$args ); } diff --git a/src/BackgroundTask/MonitorCssTransientCaching.php b/src/BackgroundTask/MonitorCssTransientCaching.php index 3e3a9b4bc53..309b4f76311 100644 --- a/src/BackgroundTask/MonitorCssTransientCaching.php +++ b/src/BackgroundTask/MonitorCssTransientCaching.php @@ -22,7 +22,7 @@ * @since 2.0 * @internal */ -final class MonitorCssTransientCaching extends CronBasedBackgroundTask { +final class MonitorCssTransientCaching extends RecurringBackgroundTask { /** * Name of the event to schedule. @@ -92,23 +92,24 @@ protected function get_event_name() { * @todo This has arbitrary arguments to allow for testing, as we don't have dependency injection for services. * With dependency injection, we could for example inject a Clock object and mock it for testing. * - * @param DateTimeInterface $date Optional. Date to use for timestamping the processing (for testing). - * @param int $transient_count Optional. Count of transients to use for the processing (for testing). + * @param array ...$args { + * Arguments passed to the cron callback. + * + * @type DateTimeInterface $date Optional. Date to use for timestamping the processing (for testing). + * @type int $transient_count Optional. Count of transients to use for the processing (for testing). + * } * @return void * @throws Exception If a date could not be instantiated. */ - public function process( DateTimeInterface $date = null, $transient_count = null ) { + public function process( ...$args ) { if ( wp_using_ext_object_cache() || $this->is_css_transient_caching_disabled() ) { return; } - if ( null === $date ) { - $date = new DateTimeImmutable(); - } + $date = isset( $args[0] ) && $args[0] instanceof DateTimeInterface ? $args[0] : new DateTimeImmutable(); - if ( null === $transient_count ) { - $transient_count = $this->query_css_transient_count(); - } + /** @phpstan-ignore-next-line */ + $transient_count = isset( $args[1] ) ? (int) $args[1] : $this->query_css_transient_count(); $date_string = $date->format( 'Ymd' ); $time_series = $this->get_time_series(); diff --git a/src/BackgroundTask/RecurringBackgroundTask.php b/src/BackgroundTask/RecurringBackgroundTask.php new file mode 100644 index 00000000000..381bb0d4303 --- /dev/null +++ b/src/BackgroundTask/RecurringBackgroundTask.php @@ -0,0 +1,55 @@ +get_event_name(), [ $this, 'process' ] ); + } + + /** + * Schedule the event. + * + * @param mixed[] ...$args Arguments passed to the function from the action hook. + */ + final public function schedule_event( ...$args ) { + if ( ! is_user_logged_in() ) { + return; + } + + $event_name = $this->get_event_name(); + $timestamp = wp_next_scheduled( $event_name, $args ); + + if ( $timestamp ) { + return; + } + + wp_schedule_event( time(), $this->get_interval(), $event_name, $args ); + } + + /** + * Get the interval to use for the event. + * + * @return string An existing interval name. Valid values are 'hourly', 'twicedaily' or 'daily'. + */ + abstract protected function get_interval(); +} diff --git a/src/BackgroundTask/SingleScheduledBackgroundTask.php b/src/BackgroundTask/SingleScheduledBackgroundTask.php new file mode 100644 index 00000000000..718fb7e1f79 --- /dev/null +++ b/src/BackgroundTask/SingleScheduledBackgroundTask.php @@ -0,0 +1,74 @@ +get_action_hook(), [ $this, 'schedule_event' ], 10, $this->get_action_hook_arg_count() ); + add_action( $this->get_event_name(), [ $this, 'process' ] ); + } + + /** + * Schedule the event. + * + * @param array ...$args Arguments passed to the function from the action hook. + */ + public function schedule_event( ...$args ) { + if ( ! $this->should_schedule_event( $args ) ) { + return; + } + + wp_schedule_single_event( $this->get_timestamp(), $this->get_event_name(), $args ); + } + + /** + * Time after which to run the event. + * + * @return int A timestamp. Defaults to the current time. + */ + protected function get_timestamp() { + return time(); + } + + /** + * Provides the number of args expected from the action hook where the event is registered. Default 1. + * + * @return int + */ + protected function get_action_hook_arg_count() { + return 1; + } + + /** + * Returns whether the event should be scheduled. + * + * @param array $args Arguments passed from the action hook where the event is to be scheduled. + * @return boolean + */ + abstract protected function should_schedule_event( $args ); + + /** + * Gets the hook on which to schedule the event. + * + * @return string The action hook name. + */ + abstract protected function get_action_hook(); +} diff --git a/src/BackgroundTask/ValidatedUrlStylesheetDataGarbageCollection.php b/src/BackgroundTask/ValidatedUrlStylesheetDataGarbageCollection.php index 45c5b8f1d4c..f6b8873fba6 100644 --- a/src/BackgroundTask/ValidatedUrlStylesheetDataGarbageCollection.php +++ b/src/BackgroundTask/ValidatedUrlStylesheetDataGarbageCollection.php @@ -24,7 +24,7 @@ * @package AmpProject\AmpWP * @internal */ -final class ValidatedUrlStylesheetDataGarbageCollection extends CronBasedBackgroundTask { +final class ValidatedUrlStylesheetDataGarbageCollection extends RecurringBackgroundTask { /** * Name of the event to schedule. @@ -58,9 +58,10 @@ protected function get_event_name() { /** * Process a single cron tick. * + * @param mixed[] ...$args Unused callback arguments. * @return void */ - public function process() { + public function process( ...$args ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable AMP_Validated_URL_Post_Type::delete_stylesheets_postmeta_batch( 100, '1 week ago' ); } } diff --git a/src/Validation/SavePostValidationEvent.php b/src/Validation/SavePostValidationEvent.php new file mode 100644 index 00000000000..ba9d18c192c --- /dev/null +++ b/src/Validation/SavePostValidationEvent.php @@ -0,0 +1,142 @@ +dev_tools_user_access = $dev_tools_user_access; + $this->url_validation_provider = $url_validation_provider; + } + + /** + * Callback for the cron action. + * + * @param mixed ...$args The args received with the action hook where the event was scheduled. + */ + public function process( ...$args ) { + $post_id = reset( $args ); + + if ( empty( $post_id ) || empty( get_post( $post_id ) ) ) { + return; + } + + $this->url_validation_provider->get_url_validation( + get_the_permalink( $post_id ), + get_post_type( $post_id ), + true + ); + } + + /** + * Get the event name. + * + * This is the "slug" of the event, not the display name. + * + * Note: the event name should be prefixed to prevent naming collisions. + * + * @return string Name of the event. + */ + protected function get_event_name() { + return self::BACKGROUND_TASK_NAME; + } + + /** + * Returns whether the event should be scheduled. + * + * @param array $args Args passed from the action hook where the event is scheduled. + * @return boolean + */ + protected function should_schedule_event( $args ) { + if ( ! is_array( $args ) || count( $args ) !== 1 ) { + return false; + } + + // Validation is performed on post save if user has dev tools on. + if ( $this->dev_tools_user_access->is_user_enabled( wp_get_current_user() ) ) { + return false; + } + + $id = reset( $args ); + if ( empty( $id ) ) { + return false; + } + + $post = get_post( $id ); + + if ( ! $post + || + wp_is_post_revision( $post ) + || + wp_is_post_autosave( $post ) + || + 'auto-draft' === $post->post_status + || + 'trash' === $post->post_status + ) { + return false; + } + + if ( ! amp_is_post_supported( $id ) ) { + return false; + } + + return true; + } + + /** + * Gets the hook on which to schedule the event. + * + * @return string The action hook name. + */ + protected function get_action_hook() { + return 'save_post'; + } +} diff --git a/src/Validation/ScannableURLProvider.php b/src/Validation/ScannableURLProvider.php index c66c039b9b4..a841858a7e8 100644 --- a/src/Validation/ScannableURLProvider.php +++ b/src/Validation/ScannableURLProvider.php @@ -15,49 +15,24 @@ * ScannableURLProvider class. * * @since 2.1 + * @internal */ final class ScannableURLProvider { /** - * Whether to include URLs that don't support AMP. + * Instance of URLScanningContext. * - * @var bool + * @var URLScanningContext */ - private $include_unsupported; - - /** - * An allowlist of conditionals to use for querying URLs. - * - * Usually, this class will query all of the templates that don't have AMP disabled. This allows inclusion based on only these conditionals. - * - * @var string[] - */ - private $include_conditionals; - - /** - * The maximum number of URLs to provide for each content type. - * - * Templates are each a separate type, like those for is_category() and is_tag(), and each post type is a type. - * - * @var int - */ - private $limit_per_type; + private $context; /** * Class constructor. * - * @param integer $limit_per_type The maximum number of URLs to validate for each type. - * @param array $include_conditionals An allowlist of conditionals to use for validation. - * @param boolean $include_unsupported Whether to include URLs that don't support AMP. + * @param URLScanningContext $context Instance of URLScanningContext. */ - public function __construct( - $limit_per_type = 20, - $include_conditionals = [], - $include_unsupported = false - ) { - $this->limit_per_type = $limit_per_type; - $this->include_conditionals = $include_conditionals; - $this->include_unsupported = $include_unsupported; + public function __construct( URLScanningContext $context ) { + $this->context = $context; } /** @@ -86,9 +61,10 @@ public function get_urls( $offset = 0 ) { [ $this, 'does_taxonomy_support_amp' ] ); $public_post_types = get_post_types( [ 'public' => true ] ); + $limit_per_type = $this->context->get_limit_per_type(); // Include one URL of each template/content type, then another URL of each type on the next iteration. - for ( $i = $offset; $i < $this->limit_per_type + $offset; $i++ ) { + for ( $i = $offset; $i < $limit_per_type + $offset; $i++ ) { // Include all public, published posts. foreach ( $public_post_types as $post_type ) { $post_ids = $this->get_posts_that_support_amp( $this->get_posts_by_type( $post_type, $i, 1 ) ); @@ -146,11 +122,13 @@ public function get_urls( $offset = 0 ) { * @return bool Whether the template is supported. */ private function is_template_supported( $template ) { + $include_conditionals = $this->context->get_include_conditionals(); + // If we received an allowlist of conditionals, this template conditional must be present in it. - if ( ! empty( $this->include_conditionals ) ) { - return in_array( $template, $this->include_conditionals, true ); + if ( ! empty( $include_conditionals ) ) { + return in_array( $template, $include_conditionals, true ); } - if ( $this->include_unsupported ) { + if ( $this->context->get_include_unsupported() ) { return true; } @@ -175,7 +153,7 @@ private function get_posts_that_support_amp( $ids ) { return []; } - if ( $this->include_unsupported ) { + if ( $this->context->get_include_unsupported() ) { return $ids; } @@ -196,7 +174,7 @@ private function get_posts_that_support_amp( $ids ) { private function get_posts_by_type( $post_type, $offset = null, $number = null ) { $args = [ 'post_type' => $post_type, - 'posts_per_page' => is_int( $number ) ? $number : $this->limit_per_type, + 'posts_per_page' => is_int( $number ) ? $number : $this->context->get_limit_per_type(), 'post_status' => 'publish', 'orderby' => 'ID', 'order' => 'DESC', @@ -231,7 +209,7 @@ private function get_author_page_urls( $offset = '', $number = '' ) { return $author_page_urls; } - $number = ! empty( $number ) ? $number : $this->limit_per_type; + $number = ! empty( $number ) ? $number : $this->context->get_limit_per_type(); foreach ( get_users( compact( 'offset', 'number' ) ) as $author ) { $author_page_urls[] = get_author_posts_url( $author->ID, $author->user_nicename ); } @@ -291,7 +269,7 @@ private function does_taxonomy_support_amp( $taxonomy ) { */ private function get_taxonomy_links( $taxonomy, $offset = '', $number = null ) { if ( is_null( $number ) ) { - $number = $this->limit_per_type; + $number = $this->context->get_limit_per_type(); } return array_map( diff --git a/src/Validation/URLScanningContext.php b/src/Validation/URLScanningContext.php new file mode 100644 index 00000000000..9b6c203bc65 --- /dev/null +++ b/src/Validation/URLScanningContext.php @@ -0,0 +1,103 @@ +limit_per_type = $limit_per_type; + $this->include_conditionals = $include_conditionals; + $this->include_unsupported = $include_unsupported; + } + + /** + * Provides the limit_per_type setting. + * + * @return int + */ + public function get_limit_per_type() { + /** + * Filters the number of URLs per content type to check during each run of the validation cron task. + * + * @since 2.1.0 + * @param int $url_validation_number_per_type The number of URLs. Defaults to 1. Filtering to -1 will result in all being returned. + */ + $url_validation_limit_per_type = (int) apply_filters( 'amp_url_validation_limit_per_type', $this->limit_per_type ); + + return max( $url_validation_limit_per_type, -1 ); + } + + /** + * Provides the include_conditionals setting. + * + * @return string[] + */ + public function get_include_conditionals() { + return $this->include_conditionals; + } + + /** + * Provides the include_unsupported setting. + * + * @return bool + */ + public function get_include_unsupported() { + return $this->include_unsupported; + } +} diff --git a/src/Validation/URLValidationCron.php b/src/Validation/URLValidationCron.php new file mode 100644 index 00000000000..e2784b7512b --- /dev/null +++ b/src/Validation/URLValidationCron.php @@ -0,0 +1,119 @@ +scannable_url_provider = $scannable_url_provider; + $this->url_validation_provider = $url_validation_provider; + } + + /** + * Callback for the cron action. + * + * @param mixed[] ...$args Unused callback arguments. + */ + public function process( ...$args ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable + $urls = $this->scannable_url_provider->get_urls(); + $sleep_time = $this->get_sleep_time(); + + foreach ( $urls as $url ) { + $this->url_validation_provider->get_url_validation( $url['url'], $url['type'], true ); + if ( $sleep_time ) { + sleep( $sleep_time ); + } + } + } + + /** + * Get the event name. + * + * This is the "slug" of the event, not the display name. + * + * Note: the event name should be prefixed to prevent naming collisions. + * + * @return string Name of the event. + */ + protected function get_event_name() { + return self::BACKGROUND_TASK_NAME; + } + + /** + * Get the interval to use for the event. + * + * @return string An existing interval name. + */ + protected function get_interval() { + return self::DEFAULT_INTERVAL_DAILY; + } + + /** + * Provides the length of time, in seconds, to sleep between validating URLs. + * + * @return int + */ + private function get_sleep_time() { + + /** + * Filters the length of time to sleep between validating URLs. + * + * @since 2.1 + * @param int The number of seconds. Default 1. Setting to 0 or a negative numbers disables all throttling. + */ + return max( (int) apply_filters( 'amp_url_validation_sleep_time', self::DEFAULT_SLEEP_TIME ), 0 ); + } +} diff --git a/src/Validation/URLValidationProvider.php b/src/Validation/URLValidationProvider.php index d3fffd39cf5..dfba84984a1 100644 --- a/src/Validation/URLValidationProvider.php +++ b/src/Validation/URLValidationProvider.php @@ -17,23 +17,9 @@ * URLValidationProvider class. * * @since 2.1 + * @internal */ final class URLValidationProvider { - - /** - * Key for the transient signaling validation is locked. - * - * @var string - */ - const LOCK_KEY = 'amp_validation_locked'; - - /** - * The length of time to keep the lock in place if a process fails to unlock. - * - * @var int - */ - const LOCK_TIMEOUT = 5 * MINUTE_IN_SECONDS; - /** * The total number of validation errors, regardless of whether they were accepted. * @@ -72,60 +58,6 @@ final class URLValidationProvider { */ private $validity_by_type = []; - /** - * Locks validation. - */ - private function lock() { - update_option( self::LOCK_KEY, time(), false ); - } - - /** - * Unlocks validation. - */ - private function unlock() { - delete_option( self::LOCK_KEY ); - } - - /** - * Returns whether validation is currently locked. - * - * @return boolean - */ - public function is_locked() { - $lock_time = (int) get_option( self::LOCK_KEY, 0 ); - - // It's locked if the difference between the lock time and the current time is less than the lockout time. - return time() - $lock_time < self::LOCK_TIMEOUT; - } - - /** - * Runs a callback with a lock set for the duration of the callback. - * - * @param callable $callback Callback to run with the lock set. - * @return mixed WP_Error if a lock is in place. Otherwise, the result of the callback or void if it doesn't return anything. - */ - public function with_lock( $callback ) { - if ( $this->is_locked() ) { - return new WP_Error( - 'amp_url_validation_locked', - __( 'URL validation cannot start right now because another process is already validating URLs. Try again in a few minutes.', 'amp' ) - ); - } - - $this->lock(); - $result = $callback(); - $this->unlock(); - - return $result; - } - - /** - * Resets the lock timeout. This allows long-running processes to keep running beyond the lock timeout. - */ - public function reset_lock() { - $this->lock(); - } - /** * Provides the total number of validation errors found. * diff --git a/tests/php/src/BackgroundTask/BackgroundTaskDeactivatorTest.php b/tests/php/src/BackgroundTask/BackgroundTaskDeactivatorTest.php new file mode 100644 index 00000000000..3555d378de2 --- /dev/null +++ b/tests/php/src/BackgroundTask/BackgroundTaskDeactivatorTest.php @@ -0,0 +1,134 @@ +test_instance = new BackgroundTaskDeactivator(); + } + + /** + * @covers ::register() + */ + public function test_register() { + $this->assertInstanceof( BackgroundTaskDeactivator::class, $this->test_instance ); + $this->assertInstanceof( Service::class, $this->test_instance ); + $this->assertInstanceof( Registerable::class, $this->test_instance ); + $this->assertInstanceof( Deactivateable::class, $this->test_instance ); + + $this->test_instance->register(); + + $plugin_file = $this->get_private_property( $this->test_instance, 'plugin_file' ); + $this->assertEquals( 'amp/amp.php', $plugin_file ); + + $this->assertEquals( 10, has_action( "network_admin_plugin_action_links_{$plugin_file}", [ $this->test_instance, 'add_warning_sign_to_network_deactivate_action' ] ) ); + $this->assertEquals( 10, has_action( 'plugin_row_meta', [ $this->test_instance, 'add_warning_to_plugin_meta' ] ) ); + } + + /** + * @covers ::add_event() + * @covers ::deactivate() + */ + public function test_deactivating_two_events() { + $original_cron = get_option( 'cron' ); + + update_option( + 'cron', + [ + time() => [ + 'event_one' => [ 'args' => [] ], + ], + time() + HOUR_IN_SECONDS => [ + 'event_two' => [ 'args' => [] ], + ], + ] + ); + + $this->assertCount( 2, _get_cron_array() ); + + $this->test_instance->add_event( 'event_one' ); + $this->test_instance->add_event( 'event_two' ); + + $this->test_instance->deactivate( false ); + $this->assertCount( 0, _get_cron_array() ); + + update_option( 'cron', $original_cron ); + } + + /** + * @covers ::add_event() + * @covers ::deactivate() + */ + public function test_deactivating_one_of_two_events() { + $original_cron = get_option( 'cron' ); + + update_option( + 'cron', + [ + time() => [ + 'event_one' => [ 'args' => [] ], + ], + time() + HOUR_IN_SECONDS => [ + 'event_two' => [ 'args' => [] ], + ], + ] + ); + + $this->assertCount( 2, _get_cron_array() ); + + $this->test_instance->add_event( 'event_one' ); + + $this->test_instance->deactivate( false ); + $this->assertCount( 1, _get_cron_array() ); + + update_option( 'cron', $original_cron ); + } + + /** + * @covers ::get_warning_icon() + * @covers ::add_warning_sign_to_network_deactivate_action() + */ + public function test_network_deactivate_warning() { + // Tested method uses multisite functions. + if ( ! function_exists( 'wp_is_large_network' ) ) { + require_once ABSPATH . WPINC . '/ms-functions.php'; + } + + add_filter( 'wp_is_large_network', '__return_true' ); + wp_register_style( 'amp-icons', 'http://site.com/file.css', [], '1' ); + + $actions = [ + 'deactivate' => '', + ]; + + $new_actions = $this->test_instance->add_warning_sign_to_network_deactivate_action( $actions ); + + $this->assertTrue( wp_style_is( 'amp-icons' ) ); + $this->assertContains( '', $new_actions['deactivate'] ); + + remove_filter( 'wp_is_large_network', '__return_true' ); + } +} diff --git a/tests/php/test-class-monitor-css-transient-caching.php b/tests/php/src/BackgroundTask/MonitorCssTransientCachingTest.php similarity index 84% rename from tests/php/test-class-monitor-css-transient-caching.php rename to tests/php/src/BackgroundTask/MonitorCssTransientCachingTest.php index adb7ae174bf..e8a94245a32 100644 --- a/tests/php/test-class-monitor-css-transient-caching.php +++ b/tests/php/src/BackgroundTask/MonitorCssTransientCachingTest.php @@ -1,19 +1,19 @@ user->create( [ 'role' => 'administrator' ] ) ); $this->assertFalse( wp_next_scheduled( MonitorCssTransientCaching::EVENT_NAME ) ); - $monitor = new MonitorCssTransientCaching(); + $monitor = new MonitorCssTransientCaching( new BackgroundTaskDeactivator() ); $monitor->schedule_event(); $timestamp = wp_next_scheduled( MonitorCssTransientCaching::EVENT_NAME ); @@ -59,10 +59,6 @@ public function test_event_gets_scheduled_and_unscheduled() { $this->assertNotFalse( $timestamp ); $this->assertInternalType( 'int', $timestamp ); $this->assertGreaterThan( 0, $timestamp ); - - $monitor->deactivate( false ); - - $this->assertFalse( wp_next_scheduled( MonitorCssTransientCaching::EVENT_NAME ) ); } /** @@ -73,7 +69,7 @@ public function test_event_gets_scheduled_and_unscheduled() { public function test_event_can_be_processed() { delete_option( MonitorCssTransientCaching::TIME_SERIES_OPTION_KEY ); - $monitor = new MonitorCssTransientCaching(); + $monitor = new MonitorCssTransientCaching( new BackgroundTaskDeactivator() ); $monitor->process(); $this->assertNotFalse( get_option( MonitorCssTransientCaching::TIME_SERIES_OPTION_KEY ) ); @@ -101,7 +97,7 @@ static function () { } ); - $monitor = new MonitorCssTransientCaching(); + $monitor = new MonitorCssTransientCaching( new BackgroundTaskDeactivator() ); // Moving average should be 0. $monitor->process( new DateTime( '2000-01-01' ), 5 ); diff --git a/tests/php/src/ValidatedUrlStylesheetDataGarbageCollectionTest.php b/tests/php/src/ValidatedUrlStylesheetDataGarbageCollectionTest.php index f4abd884b90..09036e90e4e 100644 --- a/tests/php/src/ValidatedUrlStylesheetDataGarbageCollectionTest.php +++ b/tests/php/src/ValidatedUrlStylesheetDataGarbageCollectionTest.php @@ -5,7 +5,12 @@ * @package AmpProject\AmpWP */ +namespace AmpProject\AmpWP\Tests; + +use AMP_Validated_URL_Post_Type; +use AmpProject\AmpWP\BackgroundTask\BackgroundTaskDeactivator; use AmpProject\AmpWP\BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection; +use WP_UnitTestCase; /** @coversDefaultClass \AmpProject\AmpWP\BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection */ class ValidatedUrlStylesheetDataGarbageCollectionTest extends WP_UnitTestCase { @@ -20,7 +25,7 @@ public function test_event_gets_scheduled_and_unscheduled() { wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); $this->assertFalse( wp_next_scheduled( ValidatedUrlStylesheetDataGarbageCollection::EVENT_NAME ) ); - $monitor = new ValidatedUrlStylesheetDataGarbageCollection(); + $monitor = new ValidatedUrlStylesheetDataGarbageCollection( new BackgroundTaskDeactivator() ); $monitor->schedule_event(); $timestamp = wp_next_scheduled( ValidatedUrlStylesheetDataGarbageCollection::EVENT_NAME ); @@ -28,10 +33,6 @@ public function test_event_gets_scheduled_and_unscheduled() { $this->assertNotFalse( $timestamp ); $this->assertInternalType( 'int', $timestamp ); $this->assertGreaterThan( 0, $timestamp ); - - $monitor->deactivate( false ); - - $this->assertFalse( wp_next_scheduled( ValidatedUrlStylesheetDataGarbageCollection::EVENT_NAME ) ); } /** @@ -40,7 +41,7 @@ public function test_event_gets_scheduled_and_unscheduled() { * @covers ::process() */ public function test_event_can_be_processed() { - $monitor = new ValidatedUrlStylesheetDataGarbageCollection(); + $monitor = new ValidatedUrlStylesheetDataGarbageCollection( new BackgroundTaskDeactivator() ); // Insert four weeks of validated URLs. $post_ids = []; diff --git a/tests/php/src/Validation/SavePostValidationEventTest.php b/tests/php/src/Validation/SavePostValidationEventTest.php new file mode 100644 index 00000000000..2e42f6d272a --- /dev/null +++ b/tests/php/src/Validation/SavePostValidationEventTest.php @@ -0,0 +1,166 @@ +test_instance = new SavePostValidationEvent( new BackgroundTaskDeactivator(), new UserAccess(), new URLValidationProvider() ); + $this->dev_tools_user_access = new UserAccess(); + add_filter( 'pre_http_request', [ $this, 'get_validate_response' ] ); + } + + /** @covers ::__construct() */ + public function test__construct() { + $this->assertInstanceof( SingleScheduledBackgroundTask::class, $this->test_instance ); + $this->assertInstanceof( SavePostValidationEvent::class, $this->test_instance ); + $this->assertInstanceof( Service::class, $this->test_instance ); + $this->assertInstanceof( Registerable::class, $this->test_instance ); + } + + /** + * @covers ::register() + * @covers ::get_event_name() + * @covers ::get_action_hook_arg_count() + */ + public function test_register() { + $this->test_instance->register(); + + $this->assertEquals( 10, has_action( 'save_post', [ $this->test_instance, 'schedule_event' ] ) ); + $this->assertEquals( 10, has_action( 'amp_single_post_validate', [ $this->test_instance, 'process' ] ) ); + } + + /** + * @covers ::process() + */ + public function test_process() { + $this->test_instance->process(); + $this->assertCount( 0, $this->get_validated_urls() ); + + $post = $this->factory()->post->create_and_get( + [ + 'post_content' => '
', + ] + ); + + $this->test_instance->process( $post->ID ); + + $this->assertCount( 1, $this->get_validated_urls() ); + + $this->assertInstanceof( + URLValidationProvider::class, + $this->get_private_property( $this->test_instance, 'url_validation_provider' ) + ); + } + + /** @covers ::schedule_event() */ + public function test_schedule_event_with_no_post() { + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $event_was_scheduled = false; + $filter_cb = static function ( $event ) use ( &$event_was_scheduled ) { + $event_was_scheduled = true; + return $event; + }; + add_filter( 'schedule_event', $filter_cb ); + + $this->test_instance->schedule_event( [] ); + + $this->assertFalse( $event_was_scheduled ); + + remove_filter( 'schedule_event', $filter_cb ); + } + + /** @covers ::schedule_event() */ + public function test_schedule_event_with_post() { + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->dev_tools_user_access->set_user_enabled( wp_get_current_user(), true ); + + $event_was_scheduled = false; + $filter_cb = static function ( $event ) use ( &$event_was_scheduled ) { + $event_was_scheduled = true; + return $event; + }; + add_filter( 'schedule_event', $filter_cb ); + + $post = $this->factory()->post->create(); + + $this->test_instance->schedule_event( $post ); + + $this->assertFalse( $event_was_scheduled ); + + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'author' ] ) ); + + $this->test_instance->schedule_event( $post ); + + $this->assertTrue( $event_was_scheduled ); + + remove_filter( 'schedule_event', $filter_cb ); + } + + /** @covers ::should_schedule_event() */ + public function test_should_schedule_event() { + // No user set. + $this->assertFalse( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ [] ] ) ); + + // Array not passed. + $this->assertFalse( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ null ] ) ); + + // Too many args passed. + $this->assertFalse( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ [ 'arg1', 'arg2' ] ] ) ); + + // User with insufficient permissions. + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'subscriber' ] ) ); + $post = $this->factory()->post->create(); + $this->assertTrue( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ [ $post ] ] ) ); + + // User with dev tools off. + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->dev_tools_user_access->set_user_enabled( wp_get_current_user(), false ); + $post = $this->factory()->post->create(); + $this->assertTrue( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ [ $post ] ] ) ); + + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->dev_tools_user_access->set_user_enabled( wp_get_current_user(), true ); + $post = $this->factory()->post->create(); + $this->assertFalse( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ [ $post ] ] ) ); + } + + /** @covers ::get_action_hook() */ + public function test_get_action_hook() { + $this->assertEquals( 'save_post', $this->call_private_method( $this->test_instance, 'get_action_hook' ) ); + } +} diff --git a/tests/php/src/Validation/ScannableURLProviderTest.php b/tests/php/src/Validation/ScannableURLProviderTest.php index 55499ef71dc..d6953228f5f 100644 --- a/tests/php/src/Validation/ScannableURLProviderTest.php +++ b/tests/php/src/Validation/ScannableURLProviderTest.php @@ -10,6 +10,7 @@ use AmpProject\AmpWP\Tests\Helpers\PrivateAccess; use AmpProject\AmpWP\Tests\Helpers\ValidationRequestMocking; use AmpProject\AmpWP\Validation\ScannableURLProvider; +use AmpProject\AmpWP\Validation\URLScanningContext; use WP_Query; use WP_UnitTestCase; @@ -31,7 +32,7 @@ final class ScannableURLProviderTest extends WP_UnitTestCase { */ public function setUp() { parent::setUp(); - $this->scannable_url_provider = new ScannableURLProvider(); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [], false ) ); add_filter( 'pre_http_request', [ $this, 'get_validate_response' ] ); } @@ -50,7 +51,7 @@ public function test_count_urls_to_validate() { $this->assertCount( $number_original_urls, $this->scannable_url_provider->get_urls() ); - $this->scannable_url_provider = new ScannableURLProvider( 100 ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 100 ) ); $category = self::factory()->term->create( [ 'taxonomy' => 'category' ] ); $number_new_posts = 50; @@ -70,7 +71,7 @@ public function test_count_urls_to_validate() { $expected_url_count = $number_new_posts + $number_original_urls + 1; $this->assertCount( $expected_url_count, $this->scannable_url_provider->get_urls() ); - $this->scannable_url_provider = new ScannableURLProvider( 100 ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 100 ) ); $number_of_new_terms = 20; $expected_url_count += $number_of_new_terms; @@ -126,9 +127,9 @@ public function test_get_posts_that_support_amp() { ); // When the second $force_count_all_urls argument is true, all of the newly-created posts should be part of the URL count. - $this->set_private_property( $this->scannable_url_provider, 'include_unsupported', true ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [], true ) ); $this->assertEquals( $ids, $this->call_private_method( $this->scannable_url_provider, 'get_posts_that_support_amp', [ $ids ] ) ); - $this->set_private_property( $this->scannable_url_provider, 'include_unsupported', false ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [], false ) ); // In AMP-first, the IDs should include all of the newly-created posts. AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::STANDARD_MODE_SLUG ); @@ -147,16 +148,15 @@ public function test_get_posts_that_support_amp() { * If the WP-CLI command has an include argument, and is_singular isn't in it, no posts will have AMP enabled. * For example, wp amp validate-site --include=is_tag,is_category */ - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [ 'is_tag', 'is_category' ] ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [ 'is_tag', 'is_category' ], false ) ); $this->assertEquals( [], $this->call_private_method( $this->scannable_url_provider, 'get_posts_that_support_amp', [ $ids ] ) ); /* * If is_singular is in the WP-CLI argument, it should return these posts as being AMP-enabled. * For example, wp amp validate-site include=is_singular,is_category */ - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [ 'is_singular', 'is_category' ] ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [ 'is_singular', 'is_category' ], false ) ); $this->assertEmpty( array_diff( $ids, $this->call_private_method( $this->scannable_url_provider, 'get_posts_that_support_amp', [ $ids ] ) ) ); - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [] ); } /** @@ -183,16 +183,15 @@ public function test_get_author_page_urls() { $this->assertEquals( [ $second_author_url ], $actual_urls ); // If $include_conditionals is set and does not have is_author, this should not return a URL. - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [ 'is_category' ] ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [ 'is_category' ], false ) ); $this->assertEquals( [], $this->call_private_method( $this->scannable_url_provider, 'get_author_page_urls' ) ); // If $include_conditionals is set and has is_author, this should return URLs. - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [ 'is_author' ] ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [ 'is_author' ], false ) ); $this->assertEquals( [ $first_author_url, $second_author_url ], $this->call_private_method( $this->scannable_url_provider, 'get_author_page_urls' ) ); - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [] ); } /** @@ -219,11 +218,11 @@ public function test_does_taxonomy_support_amp() { } // When $include_unsupported is true, all taxonomies should be supported. - $this->set_private_property( $this->scannable_url_provider, 'include_unsupported', true ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [], true ) ); foreach ( $taxonomies_to_test as $taxonomy ) { $this->assertTrue( $this->call_private_method( $this->scannable_url_provider, 'does_taxonomy_support_amp', [ $taxonomy ] ) ); } - $this->set_private_property( $this->scannable_url_provider, 'include_unsupported', false ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [], false ) ); // When the user has checked the Option::ALL_TEMPLATES_SUPPORTED box, this should always be true. AMP_Options_Manager::update_option( Option::ALL_TEMPLATES_SUPPORTED, true ); @@ -236,12 +235,11 @@ public function test_does_taxonomy_support_amp() { * If the user passed allowed conditionals to the WP-CLI command like wp amp validate-site --include=is_category,is_tag * these should be supported taxonomies. */ - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [ 'is_category', 'is_tag' ] ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [ 'is_category', 'is_tag' ], true ) ); $this->assertTrue( $this->call_private_method( $this->scannable_url_provider, 'does_taxonomy_support_amp', [ 'category' ] ) ); $this->assertTrue( $this->call_private_method( $this->scannable_url_provider, 'does_taxonomy_support_amp', [ 'tag' ] ) ); $this->assertFalse( $this->call_private_method( $this->scannable_url_provider, 'does_taxonomy_support_amp', [ 'author' ] ) ); $this->assertFalse( $this->call_private_method( $this->scannable_url_provider, 'does_taxonomy_support_amp', [ 'search' ] ) ); - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [] ); } /** @@ -365,13 +363,12 @@ public function test_get_search_page() { $this->assertTrue( is_string( $this->call_private_method( $this->scannable_url_provider, 'get_search_page' ) ) ); // If $include_conditionals is set and does not have is_search, this should not return a URL. - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [ 'is_author' ] ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [ 'is_author' ], false ) ); $this->assertEquals( null, $this->call_private_method( $this->scannable_url_provider, 'get_search_page' ) ); // If $include_conditionals has is_search, this should return a URL. - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [ 'is_search' ] ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [ 'is_search' ], false ) ); $this->assertTrue( is_string( $this->call_private_method( $this->scannable_url_provider, 'get_search_page' ) ) ); - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [] ); } /** @@ -386,13 +383,12 @@ public function test_get_date_page() { $this->assertStringContains( $year, $this->call_private_method( $this->scannable_url_provider, 'get_date_page' ) ); // If $include_conditionals is set and does not have is_date, this should not return a URL. - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [ 'is_search' ] ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [ 'is_search' ], false ) ); $this->assertEquals( null, $this->call_private_method( $this->scannable_url_provider, 'get_date_page' ) ); // If $include_conditionals has is_date, this should return a URL. - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [ 'is_date' ] ); - $parsed_page_url = wp_parse_url( $this->call_private_method( $this->scannable_url_provider, 'get_date_page' ) ); + $this->scannable_url_provider = new ScannableURLProvider( new URLScanningContext( 20, [ 'is_date' ], false ) ); + $parsed_page_url = wp_parse_url( $this->call_private_method( $this->scannable_url_provider, 'get_date_page' ) ); $this->assertStringContains( $year, $parsed_page_url['query'] ); - $this->set_private_property( $this->scannable_url_provider, 'include_conditionals', [] ); } } diff --git a/tests/php/src/Validation/URLScanningContextTest.php b/tests/php/src/Validation/URLScanningContextTest.php new file mode 100644 index 00000000000..176e6b19754 --- /dev/null +++ b/tests/php/src/Validation/URLScanningContextTest.php @@ -0,0 +1,81 @@ +test_instance = new URLScanningContext(); + } + + /** + * @covers ::__construct() + */ + public function test__construct() { + $this->assertInstanceof( URLScanningContext::class, $this->test_instance ); + } + + /** @covers ::get_limit_per_type() */ + public function test_get_limit_per_type() { + $this->assertEquals( 1, $this->test_instance->get_limit_per_type() ); + + add_filter( + 'amp_url_validation_limit_per_type', + static function() { + return -4; + } + ); + $this->assertEquals( -1, $this->test_instance->get_limit_per_type() ); + + add_filter( + 'amp_url_validation_limit_per_type', + static function() { + return -1; + } + ); + $this->assertEquals( -1, $this->test_instance->get_limit_per_type() ); + + add_filter( + 'amp_url_validation_limit_per_type', + static function() { + return 0; + } + ); + $this->assertEquals( 0, $this->test_instance->get_limit_per_type() ); + } + + /** + * @covers ::get_limit_per_type() + * @covers ::get_include_conditionals() + * @covers ::get_include_unsupported() + */ + public function test_getters() { + $this->test_instance = new URLScanningContext( 99, [ 'is_date', 'is_search' ], true ); + + $this->assertEquals( 99, $this->test_instance->get_limit_per_type() ); + $this->assertEquals( + [ 'is_date', 'is_search' ], + $this->test_instance->get_include_conditionals() + ); + $this->assertTrue( $this->test_instance->get_include_unsupported() ); + } +} diff --git a/tests/php/src/Validation/URLValidationCronTest.php b/tests/php/src/Validation/URLValidationCronTest.php new file mode 100644 index 00000000000..b6d16abe5ae --- /dev/null +++ b/tests/php/src/Validation/URLValidationCronTest.php @@ -0,0 +1,119 @@ +test_instance = new URLValidationCron( new BackgroundTaskDeactivator(), new ScannableURLProvider( new URLScanningContext( 20 ) ), new URLValidationProvider() ); + add_filter( 'pre_http_request', [ $this, 'get_validate_response' ] ); + } + + /** + * @covers ::register() + * @covers ::get_event_name() + */ + public function test_register() { + $this->assertInstanceof( CronBasedBackgroundTask::class, $this->test_instance ); + $this->assertInstanceof( URLValidationCron::class, $this->test_instance ); + $this->assertInstanceof( Service::class, $this->test_instance ); + $this->assertInstanceof( Registerable::class, $this->test_instance ); + + $this->test_instance->register(); + + $this->assertEquals( 10, has_action( 'admin_init', [ $this->test_instance, 'schedule_event' ] ) ); + $this->assertEquals( 10, has_action( URLValidationCron::BACKGROUND_TASK_NAME, [ $this->test_instance, 'process' ] ) ); + } + + /** @covers ::schedule_event() */ + public function test_schedule_event_with_no_user() { + $event_name = $this->call_private_method( $this->test_instance, 'get_event_name' ); + + // No logged-in user. + $this->test_instance->schedule_event(); + + $this->assertFalse( wp_next_scheduled( $event_name ) ); + } + + /** @covers ::schedule_event() */ + public function test_schedule_event_with_user_without_permission() { + $event_name = $this->call_private_method( $this->test_instance, 'get_event_name' ); + + $this->assertFalse( wp_next_scheduled( $event_name ) ); + + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'subscriber' ] ) ); + + $this->test_instance->schedule_event(); + + $this->assertTrue( is_numeric( wp_next_scheduled( $event_name ) ) ); + } + + /** @covers ::schedule_event() */ + public function test_schedule_event_with_user_with_permission() { + $event_name = $this->call_private_method( $this->test_instance, 'get_event_name' ); + + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $this->test_instance->schedule_event(); + + $this->assertNotFalse( wp_next_scheduled( $event_name ) ); + } + + /** + * Test validate_urls. + * + * @covers ::process() + * @covers ::get_sleep_time() + */ + public function test_validate_urls() { + $this->factory()->post->create_many( 5 ); + + add_filter( 'amp_url_validation_sleep_time', '__return_false' ); + + $this->test_instance->process(); + $this->assertCount( 10, $this->get_validated_urls() ); + } + + /** @covers ::get_event_name() */ + public function test_get_event_name() { + $this->assertEquals( + URLValidationCron::BACKGROUND_TASK_NAME, + $this->call_private_method( $this->test_instance, 'get_event_name' ) + ); + } + + /** @covers ::get_interval() */ + public function test_get_interval() { + $this->assertEquals( + URLValidationCron::DEFAULT_INTERVAL_DAILY, + $this->call_private_method( $this->test_instance, 'get_interval' ) + ); + } +} diff --git a/tests/php/src/Validation/URLValidationProviderTest.php b/tests/php/src/Validation/URLValidationProviderTest.php index ec7388186e4..f3600c52e89 100644 --- a/tests/php/src/Validation/URLValidationProviderTest.php +++ b/tests/php/src/Validation/URLValidationProviderTest.php @@ -63,32 +63,4 @@ public function test_get_url_validation() { array_keys( $this->url_validation_provider->get_validity_by_type() ) ); } - - /** - * Tests locking and unlocking. - * - * @covers ::lock() - * @covers ::unlock() - * @covers ::is_locked() - * @covers ::with_lock() - */ - public function test_locking() { - $this->assertFalse( get_option( URLValidationProvider::LOCK_KEY ) ); - - $expected_result = 'EXPECTED RESULT'; - $result = $this->url_validation_provider->with_lock( - function () use ( $expected_result ) { - $this->assertTrue( (bool) get_option( URLValidationProvider::LOCK_KEY ) ); - - // Expect an error when lock is already in place. - $this->assertWPError( $this->url_validation_provider->with_lock( '__return_empty_string' ) ); - - return $expected_result; - } - ); - - $this->assertEquals( $expected_result, $result ); - - $this->assertFalse( get_option( URLValidationProvider::LOCK_KEY ) ); - } }