Merge pull request #6520 from ampproject/dev-tool/5750-refactor-url-v…

…alidation-cron

Refactor cron event to validate individual URLs

.phpstorm.meta.php

@@ -60,6 +60,8 @@
 			'url_validation_cron'                => \AmpProject\AmpWP\Validation\URLValidationCron::class,
 			'url_validation_rest_controller'     => \AmpProject\AmpWP\Validation\URLValidationRESTController::class,
 			'validated_url_stylesheet_gc'        => \AmpProject\AmpWP\BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection::class,
+			'validation.scannable_url_provider'  => \AmpProject\AmpWP\Validation\ScannableURLProvider::class,
+			'validation.url_validation_provider' => \AmpProject\AmpWP\Validation\URLValidationProvider::class,
 		] )
 	);
 

assets/src/onboarding-wizard/pages/done/index.js

@@ -47,7 +47,14 @@ export function Done() {
 	const { didSaveDeveloperToolsOption, saveDeveloperToolsOption, savingDeveloperToolsOption } = useContext( User );
 	const { canGoForward, setCanGoForward } = useContext( Navigation );
 	const { downloadedTheme, downloadingTheme, downloadingThemeError } = useContext( ReaderThemes );
-	const { previewLinks, setActivePreviewLink, previewUrl, isPreviewingAMP, toggleIsPreviewingAMP } = usePreview();
+	const {
+		hasPreview,
+		isPreviewingAMP,
+		previewLinks,
+		previewUrl,
+		setActivePreviewLink,
+		toggleIsPreviewingAMP,
+	} = usePreview();
 
 	/**
 	 * Allow the finish button to be enabled.
@@ -104,55 +111,75 @@ export function Done() {
 					{ __( 'Your site is ready to bring great experiences to your users!', 'amp' ) }
 				</p>
 				{ STANDARD === themeSupport && (
-					<p>
-						{ __( 'In Standard mode there is a single AMP version of your site. Browse your site here to ensure it meets your expectations.', 'amp' ) }
-					</p>
+					<>
+						<p>
+							{ __( 'In Standard mode there is a single AMP version of your site.', 'amp' ) }
+						</p>
+						{ hasPreview && (
+							<p>
+								{ __( 'Browse your site here to ensure it meets your expectations.', 'amp' ) }
+							</p>
+						) }
+					</>
 				) }
 				{ TRANSITIONAL === themeSupport && (
 					<>
 						<p>
 							{ __( 'In Transitional mode AMP and non-AMP versions of your site are served using your currently active theme.', 'amp' ) }
 						</p>
-						<p>
-							{ __( 'Browse your site here to ensure it meets your expectations, and toggle the AMP setting to compare both versions.', 'amp' ) }
-						</p>
+						{ hasPreview && (
+							<p>
+								{ __( 'Browse your site here to ensure it meets your expectations, and toggle the AMP setting to compare both versions.', 'amp' ) }
+							</p>
+						) }
 					</>
 				) }
 				{ READER === themeSupport && (
 					<>
 						<p>
-							{ __( 'In Reader mode AMP is served using your selected Reader theme, and pages for your non-AMP site are served using your primary theme. Browse your site here to ensure it meets your expectations, and toggle the AMP setting to compare both versions.', 'amp' ) }
+							{ __( 'In Reader mode AMP is served using your selected Reader theme, and pages for your non-AMP site are served using your primary theme.', 'amp' ) }
 						</p>
+						{ hasPreview && (
+							<p>
+								{ __( 'Browse your site here to ensure it meets your expectations, and toggle the AMP setting to compare both versions.', 'amp' ) }
+							</p>
+						) }
 						<p>
 							{ __( 'As a last step, use the Customizer to tailor the Reader theme as needed.', 'amp' ) }
 						</p>
 					</>
 				) }
-				<div className="done__links-container">
-					<NavMenu
-						links={ previewLinks }
-						onClick={ ( e, link ) => {
-							e.preventDefault();
-							setActivePreviewLink( link );
-						} }
-					/>
-				</div>
+				{ hasPreview && (
+					<div className="done__links-container">
+						<NavMenu
+							links={ previewLinks }
+							onClick={ ( e, link ) => {
+								e.preventDefault();
+								setActivePreviewLink( link );
+							} }
+						/>
+					</div>
+				) }
 			</div>
 			<div className="done__preview-container">
 				{ READER === themeSupport && downloadingThemeError && (
 					<AMPNotice size={ NOTICE_SIZE_LARGE } type={ NOTICE_TYPE_INFO }>
 						{ __( 'There was an error downloading your Reader theme. As a result, your site is currently using the legacy reader theme. Please install your chosen theme manually.', 'amp' ) }
 					</AMPNotice>
 				) }
-				{ STANDARD !== themeSupport && (
-					<AMPSettingToggle
-						text={ __( 'AMP', 'amp' ) }
-						checked={ isPreviewingAMP }
-						onChange={ toggleIsPreviewingAMP }
-						compact={ true }
-					/>
+				{ hasPreview && (
+					<>
+						{ STANDARD !== themeSupport && (
+							<AMPSettingToggle
+								text={ __( 'AMP', 'amp' ) }
+								checked={ isPreviewingAMP }
+								onChange={ toggleIsPreviewingAMP }
+								compact={ true }
+							/>
+						) }
+						<Preview url={ previewUrl } />
+					</>
 				) }
-				<Preview url={ previewUrl } />
 			</div>
 			<div className="done__content done__content--secondary">
 				<h2 className="done__icon-title">

assets/src/onboarding-wizard/pages/done/use-preview.js

@@ -15,9 +15,11 @@ import { Options } from '../../../components/options-context-provider';
 import { STANDARD } from '../../../common/constants';
 
 export function usePreview() {
+	const hasPreview = PREVIEW_URLS.length > 0;
+
 	const { editedOptions: { theme_support: themeSupport } } = useContext( Options );
 	const [ isPreviewingAMP, setIsPreviewingAMP ] = useState( themeSupport !== STANDARD );
-	const [ previewedPageType, setPreviewedPageType ] = useState( PREVIEW_URLS[ 0 ].type );
+	const [ previewedPageType, setPreviewedPageType ] = useState( hasPreview ? PREVIEW_URLS[ 0 ].type : null );
 
 	const toggleIsPreviewingAMP = () => setIsPreviewingAMP( ( mode ) => ! mode );
 	const setActivePreviewLink = ( link ) => setPreviewedPageType( link.type );
@@ -32,10 +34,11 @@ export function usePreview() {
 	const previewUrl = useMemo( () => previewLinks.find( ( link ) => link.isActive )?.url, [ previewLinks ] );
 
 	return {
+		hasPreview,
+		isPreviewingAMP,
 		previewLinks,
-		setActivePreviewLink,
 		previewUrl,
-		isPreviewingAMP,
+		setActivePreviewLink,
 		toggleIsPreviewingAMP,
 	};
 }

assets/src/settings-page/site-review.js

@@ -40,7 +40,7 @@ export function SiteReview() {
 		return null;
 	}
 
-	const previewPermalink = STANDARD === themeSupport ? HOME_URL : pairedUrlExamples[ pairedUrlStructure ][ 0 ];
+	const previewPermalink = STANDARD === themeSupport ? HOME_URL : pairedUrlExamples?.[ pairedUrlStructure ]?.[ 0 ] ?? HOME_URL;
 
 	return (
 		<AMPDrawer

includes/admin/functions.php

@@ -32,6 +32,9 @@ function amp_init_customizer() {
 /**
  * Get permalink for the first AMP-eligible post.
  *
+ * @todo Eliminate this in favor of ScannableURLProvider::get_posts_by_type().
+ * @see \AmpProject\AmpWP\Validation\ScannableURLProvider::get_posts_by_type()
+ *
  * @internal
  * @return string|null URL on success, null if none found.
  */

includes/class-amp-theme-support.php

@@ -686,9 +686,15 @@ public static function get_template_availability( $query = null ) {
 	/**
 	 * Get the templates which can be supported.
 	 *
+	 * @param array $options Optional AMP options to override what has been saved.
 	 * @return array Supportable templates.
 	 */
-	public static function get_supportable_templates() {
+	public static function get_supportable_templates( $options = [] ) {
+		$options = array_merge(
+			AMP_Options_Manager::get_options(),
+			$options
+		);
+
 		$templates = [
 			'is_singular' => [
 				'label' => __( 'Singular', 'amp' ),
@@ -798,8 +804,8 @@ public static function get_supportable_templates() {
 		 */
 		$templates = apply_filters( 'amp_supportable_templates', $templates );
 
-		$supported_templates = AMP_Options_Manager::get_option( Option::SUPPORTED_TEMPLATES );
-		$are_all_supported   = AMP_Options_Manager::get_option( Option::ALL_TEMPLATES_SUPPORTED );
+		$supported_templates = $options[ Option::SUPPORTED_TEMPLATES ];
+		$are_all_supported   = $options[ Option::ALL_TEMPLATES_SUPPORTED ];
 
 		$did_filter_supply_supported = false;
 		$did_filter_supply_immutable = false;

includes/uninstall-functions.php

@@ -19,6 +19,7 @@ function delete_options() {
 	delete_option( 'amp-options' );
 	delete_option( 'amp_css_transient_monitor_time_series' );
 	delete_option( 'amp_customize_setting_modified_timestamps' );
+	delete_option( 'amp_url_validation_queue' ); // See Validation\URLValidationCron::OPTION_KEY.
 
 	$theme_mod_name = 'amp_customize_setting_modified_timestamps';
 	remove_theme_mod( $theme_mod_name );

includes/validation/class-amp-validated-url-post-type.php

@@ -1084,9 +1084,16 @@ public static function get_validated_environment() {
 		return [
 			'theme'   => $theme,
 			'plugins' => wp_list_pluck( $plugin_registry->get_plugins( true, false ), 'Version' ), // @todo What about multiple plugins being in the same directory?
-			'options' => [
-				Option::THEME_SUPPORT => AMP_Options_Manager::get_option( Option::THEME_SUPPORT ),
-			],
+			'options' => wp_array_slice_assoc(
+				AMP_Options_Manager::get_options(),
+				[
+					Option::ALL_TEMPLATES_SUPPORTED,
+					Option::READER_THEME,
+					Option::SUPPORTED_POST_TYPES,
+					Option::SUPPORTED_TEMPLATES,
+					Option::THEME_SUPPORT,
+				]
+			),
 		];
 	}
 
@@ -1149,7 +1156,14 @@ public static function get_post_staleness( $post ) {
 		} else {
 			$old_options = [];
 		}
-		$option_differences = array_diff_assoc( $old_options, $new_validated_environment['options'] );
+		$option_differences = [];
+		foreach ( $new_validated_environment['options'] as $option => $value ) {
+			if ( ! isset( $old_options[ $option ] ) ) {
+				$option_differences[ $option ] = null;
+			} elseif ( $old_options[ $option ] !== $value ) {
+				$option_differences[ $option ] = $old_options[ $option ];
+			}
+		}
 		if ( ! empty( $option_differences ) ) {
 			$staleness['options'] = $option_differences;
 		}

src/AmpWpPlugin.php

@@ -20,8 +20,10 @@
 use AmpProject\AmpWP\Support\SupportCliCommand;
 use AmpProject\AmpWP\Support\SupportRESTController;
 use AmpProject\AmpWP\Validation\SavePostValidationEvent;
+use AmpProject\AmpWP\Validation\ScannableURLProvider;
 use AmpProject\AmpWP\Validation\URLValidationCron;
 use AmpProject\AmpWP\BackgroundTask\BackgroundTaskDeactivator;
+use AmpProject\AmpWP\Validation\URLValidationProvider;
 use AmpProject\Optimizer;
 
 use AmpProject\RemoteGetRequest;
@@ -121,6 +123,8 @@ final class AmpWpPlugin extends ServiceBasedPlugin {
 		'url_validation_cron'                => URLValidationCron::class,
 		'url_validation_rest_controller'     => Validation\URLValidationRESTController::class,
 		'validated_url_stylesheet_gc'        => BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection::class,
+		'validation.scannable_url_provider'  => ScannableURLProvider::class,
+		'validation.url_validation_provider' => URLValidationProvider::class,
 	];
 
 	/**

src/BackgroundTask/RecurringBackgroundTask.php

@@ -37,13 +37,86 @@ final public function schedule_event( ...$args ) {
 		}
 
 		$event_name = $this->get_event_name();
-		$timestamp  = wp_next_scheduled( $event_name, $args );
+		$recurrence = $this->get_interval();
 
-		if ( $timestamp ) {
-			return;
+		$scheduled_event = $this->get_scheduled_event( $event_name, $args );
+
+		// Unschedule any existing event which had a differing recurrence.
+		if ( $scheduled_event && $scheduled_event->schedule !== $recurrence ) {
+			wp_unschedule_event( $scheduled_event->timestamp, $event_name, $args );
+			$scheduled_event = null;
+		}
+
+		if ( ! $scheduled_event ) {
+			wp_schedule_event( time(), $recurrence, $event_name, $args );
+		}
+	}
+
+	/**
+	 * Retrieve a scheduled event.
+	 *
+	 * This uses a copied implementation from WordPress core if `wp_get_scheduled_event()` does not exist, as it was
+	 * introduced in WordPress 5.1.
+	 *
+	 * @link https://github.com/WordPress/wordpress-develop/blob/ba943e113d3b31b121f7/src/wp-includes/cron.php#L753-L793
+	 * @see \wp_get_scheduled_event()
+	 * @codeCoverageIgnore
+	 *
+	 * @param string   $hook      Action hook of the event.
+	 * @param array    $args      Optional. Array containing each separate argument to pass to the hook's callback function.
+	 *                            Although not passed to a callback, these arguments are used to uniquely identify the
+	 *                            event, so they should be the same as those used when originally scheduling the event.
+	 *                            Default empty array.
+	 * @param int|null $timestamp Optional. Unix timestamp (UTC) of the event. If not specified, the next scheduled event
+	 *                            is returned. Default null.
+	 * @return object|false The event object. False if the event does not exist.
+	 */
+	final public function get_scheduled_event( $hook, $args = [], $timestamp = null ) {
+		if ( function_exists( 'wp_get_scheduled_event' ) ) {
+			return wp_get_scheduled_event( $hook, $args, $timestamp );
+		}
+
+		if ( null !== $timestamp && ! is_numeric( $timestamp ) ) {
+			return false;
+		}
+
+		$crons = _get_cron_array();
+		if ( empty( $crons ) ) {
+			return false;
+		}
+
+		$key = md5( serialize( $args ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize -- This is copied from WP core.
+
+		if ( ! $timestamp ) {
+			// Get next event.
+			$next = false;
+			foreach ( $crons as $timestamp => $cron ) {
+				if ( isset( $cron[ $hook ][ $key ] ) ) {
+					$next = $timestamp;
+					break;
+				}
+			}
+			if ( ! $next ) {
+				return false;
+			}
+
+			$timestamp = $next;
+		} elseif ( ! isset( $crons[ $timestamp ][ $hook ][ $key ] ) ) {
+			return false;
+		}
+
+		$event = (object) [
+			'hook'      => $hook,
+			'timestamp' => $timestamp,
+			'schedule'  => $crons[ $timestamp ][ $hook ][ $key ]['schedule'],
+			'args'      => $args,
+		];
+
+		if ( isset( $crons[ $timestamp ][ $hook ][ $key ]['interval'] ) ) {
+			$event->interval = $crons[ $timestamp ][ $hook ][ $key ]['interval'];
 		}
 
-		wp_schedule_event( time(), $this->get_interval(), $event_name, $args );
+		return $event;
 	}
 
 	/**