Skip to content
This repository has been archived by the owner on Dec 9, 2023. It is now read-only.

Commit

Permalink
Recheck the validation status of a post on frontend after saving when…
Browse files Browse the repository at this point in the history
… possible

* Move validation logic from theme support to validation utils
* Eliminate saving of validation status post when doing GET request; only store validation errors by invoker.
  • Loading branch information
westonruter committed Mar 8, 2018
1 parent 0efc859 commit da5f4ad
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 62 deletions.
39 changes: 8 additions & 31 deletions includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -984,12 +984,11 @@ public static function prepare_response( $response, $args = array() ) {

$args = array_merge(
array(
'content_max_width' => ! empty( $content_width ) ? $content_width : AMP_Post_Template::CONTENT_MAX_WIDTH, // Back-compat.
'use_document_element' => true,
'validation_error_callback' => null,
'allow_dirty_styles' => self::is_customize_preview_iframe(), // Dirty styles only needed when editing (e.g. for edit shortcodes).
'allow_dirty_scripts' => is_customize_preview(), // Scripts are always needed to inject changeset UUID.
'disable_invalid_removal' => $is_validation_debug_mode,
'content_max_width' => ! empty( $content_width ) ? $content_width : AMP_Post_Template::CONTENT_MAX_WIDTH, // Back-compat.
'use_document_element' => true,
'allow_dirty_styles' => self::is_customize_preview_iframe(), // Dirty styles only needed when editing (e.g. for edit shortcodes).
'allow_dirty_scripts' => is_customize_preview(), // Scripts are always needed to inject changeset UUID.
'disable_invalid_removal' => $is_validation_debug_mode,
),
$args
);
Expand Down Expand Up @@ -1025,31 +1024,9 @@ public static function prepare_response( $response, $args = array() ) {
}

if ( AMP_Validation_Utils::should_validate_front_end() ) {

AMP_Validation_Utils::send_validation_errors_header();

// @todo Add this as a method in AMP_Validation_Utils.
// Add comment with validation errors.
$report = "\n# Validation Status\n";
$report .= "\n## Summary\n";
$report .= wp_json_encode( AMP_Validation_Utils::summarize_validation_errors( AMP_Validation_Utils::$validation_errors ), 128 /* JSON_PRETTY_PRINT */ ) . "\n";
$report .= "\n## Details\n";
$report .= wp_json_encode( AMP_Validation_Utils::$validation_errors, 128 /* JSON_PRETTY_PRINT */ ) . "\n";
$comment = $dom->createComment( $report );
$body = $dom->getElementsByTagName( 'body' )->item( 0 );
if ( $body ) {
$body->appendChild( $comment );
}

// Store validation errors if not in debug mode (since debug mode will skew validation results).
if ( ! $is_validation_debug_mode ) {
AMP_Validation_Utils::remove_source_comments( $dom );
$url = preg_replace( '#^(https?://.+?)(/.*)$#', '$1', home_url( '/' ) );
if ( isset( $_SERVER['REQUEST_URI'] ) ) {
$url .= wp_unslash( $_SERVER['REQUEST_URI'] );
}
AMP_Validation_Utils::store_validation_errors( AMP_Validation_Utils::$validation_errors, $url );
}
AMP_Validation_Utils::finalize_validation( $dom, array(
'remove_source_comments' => ! $is_validation_debug_mode,
) );
}

$response = "<!DOCTYPE html>\n";
Expand Down
182 changes: 152 additions & 30 deletions includes/utils/class-amp-validation-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ class AMP_Validation_Utils {
*/
const DEBUG_QUERY_VAR = 'amp_debug';

/**
* Query var for cache-busting.
*
* @var string
*/
const CACHE_BUST_QUERY_VAR = 'amp_cache_bust';

/**
* The slug of the post type to store AMP errors.
*
Expand Down Expand Up @@ -170,6 +177,13 @@ class AMP_Validation_Utils {
*/
public static $enqueued_style_sources = array();

/**
* Post IDs for posts that have been updated which need to be re-validated.
*
* @var int[]
*/
public static $posts_pending_frontend_validation = array();

/**
* Add the actions.
*
Expand All @@ -180,6 +194,7 @@ public static function init() {
add_action( 'init', array( __CLASS__, 'register_post_type' ) );
add_filter( 'dashboard_glance_items', array( __CLASS__, 'filter_dashboard_glance_items' ) );
add_action( 'rightnow_end', array( __CLASS__, 'print_dashboard_glance_styles' ) );
add_action( 'save_post', array( __CLASS__, 'handle_save_post_prompting_validation' ), 10, 2 );
}

add_action( 'edit_form_top', array( __CLASS__, 'print_edit_form_validation_status' ), 10, 2 );
Expand Down Expand Up @@ -274,6 +289,60 @@ public static function add_validation_hooks() {
add_filter( 'amp_content_sanitizers', array( __CLASS__, 'add_validation_callback' ) );
}

/**
* Handle save_post action to queue re-validation of the post on the frontend.
*
* @see AMP_Validation_Utils::validate_queued_posts_on_frontend()
* @param int $post_id Post ID.
* @param WP_Post $post Post.
*/
public static function handle_save_post_prompting_validation( $post_id, $post ) {
$should_validate_post = (
is_post_type_viewable( $post->post_type )
&&
! wp_is_post_autosave( $post )
&&
! wp_is_post_revision( $post )
);
if ( $should_validate_post ) {
self::$posts_pending_frontend_validation[] = $post_id;

// The reason for shutdown is to ensure that all postmeta changes have been saved, including whether AMP is enabled.
if ( ! has_action( 'shutdown', array( __CLASS__, 'validate_queued_posts_on_frontend' ) ) ) {
add_action( 'shutdown', array( __CLASS__, 'validate_queued_posts_on_frontend' ) );
}
}
}

/**
* Validate the posts pending frontend validation.
*
* @see AMP_Validation_Utils::handle_save_post_prompting_validation()
*/
public static function validate_queued_posts_on_frontend() {
$posts = array_filter(
array_map( 'get_post', self::$posts_pending_frontend_validation ),
function( $post ) {
return $post && post_supports_amp( $post ) && 'trash' !== $post->post_status;
}
);

// @todo Only validate the first and then queue the rest in WP Cron?
foreach ( $posts as $post ) {
$url = amp_get_permalink( $post->ID );
if ( ! $url ) {
continue;
}

$validation_errors = self::validate_url( $url );
if ( is_wp_error( $validation_errors ) ) {
continue;
}

self::store_validation_errors( $validation_errors, $url );
}
}

/**
* Processes markup, to determine AMP validity.
*
Expand Down Expand Up @@ -397,7 +466,6 @@ public static function summarize_validation_errors( $validation_errors ) {
$removed_attributes[ $validation_error['node_name'] ] += 1;
}

// @todo It would be best if the invalid source was tied to the invalid elements and attributes.
if ( ! empty( $validation_error['sources'] ) ) {
$source = array_pop( $validation_error['sources'] );

Expand Down Expand Up @@ -438,12 +506,11 @@ public static function reset_validation_results() {
*
* If it's not valid AMP, it displays an error message above the 'Classic' editor.
*
* @todo There needs to be a save_post action (as there was previously) to do a loopback request for the $url to update the validation results.
* @param WP_Post $post The updated post.
* @return void
*/
public static function print_edit_form_validation_status( $post ) {
if ( ! post_supports_amp( $post ) ) {
if ( ! post_supports_amp( $post ) || ! self::has_cap() ) {
return;
}

Expand Down Expand Up @@ -490,6 +557,9 @@ public static function print_edit_form_validation_status( $post ) {
);
}
if ( $url ) {
if ( $validation_status_post ) {
echo ' | ';
}
echo sprintf(
' <a href="%s" aria-label="%s" target="_blank">%s</a>',
esc_url( self::get_debug_url( $url ) ),
Expand Down Expand Up @@ -866,6 +936,50 @@ public static function should_validate_front_end() {
return self::has_cap() && isset( $_GET[ self::VALIDATE_QUERY_VAR ] ); // WPCS: CSRF ok.
}

/**
* Finalize validation.
*
* @param DOMDocument $dom Document.
* @param array $args {
* Args.
*
* @type bool $remove_source_comments Whether source comments should be removed. Defaults to true.
* @type bool $send_validation_errors_header Whether the X-AMP-Validation-Errors header should be sent. Defaults to true.
* @type bool $append_validation_status_comment Whether the validation errors should be appended as an HTML comment. Defaults to true.
* }
*/
public static function finalize_validation( DOMDocument $dom, $args = array() ) {
$args = array_merge(
array(
'send_validation_errors_header' => true,
'remove_source_comments' => true,
'append_validation_status_comment' => true,
),
$args
);

if ( $args['send_validation_errors_header'] && ! headers_sent() ) {
self::send_validation_errors_header();
}

if ( $args['remove_source_comments'] ) {
self::remove_source_comments( $dom );
}

if ( $args['append_validation_status_comment'] ) {
$report = "\n# Validation Status\n";
$report .= "\n## Summary\n";
$report .= wp_json_encode( self::summarize_validation_errors( self::$validation_errors ), 128 /* JSON_PRETTY_PRINT */ ) . "\n";
$report .= "\n## Details\n";
$report .= wp_json_encode( self::$validation_errors, 128 /* JSON_PRETTY_PRINT */ ) . "\n";
$comment = $dom->createComment( $report );
$body = $dom->getElementsByTagName( 'body' )->item( 0 );
if ( $body ) {
$body->appendChild( $comment );
}
}
}

/**
* Adds the validation callback if front-end validation is needed.
*
Expand Down Expand Up @@ -932,16 +1046,6 @@ public static function send_validation_errors_header() {
* @global WP $wp
*/
public static function store_validation_errors( $validation_errors, $url ) {

// Remove query vars that are only used to initiate validation requests.
$url = remove_query_arg(
array(
self::VALIDATE_QUERY_VAR,
self::DEBUG_QUERY_VAR,
),
$url
);

$post_for_this_url = self::get_validation_status_post( $url );

// Since there are no validation errors and there is an existing $existing_post_id, just delete the post.
Expand Down Expand Up @@ -1020,6 +1124,7 @@ public static function validate_after_plugin_activation() {
}
$validation_errors = self::validate_url( $url );
if ( is_array( $validation_errors ) && count( $validation_errors ) > 0 ) {
self::store_validation_errors( $validation_errors, $url );
set_transient( self::PLUGIN_ACTIVATION_VALIDATION_ERRORS_TRANSIENT_KEY, $validation_errors, 60 );
} else {
delete_transient( self::PLUGIN_ACTIVATION_VALIDATION_ERRORS_TRANSIENT_KEY );
Expand All @@ -1033,24 +1138,34 @@ public static function validate_after_plugin_activation() {
* The validation errors will be stored in the validation status custom post type,
* as well as in a transient.
*
* @todo Instead of storing validation errors in process_response, it should be done here instead.
* @param string $url The URL to validate.
* @return array|WP_Error The validation errors, or WP_Error on error.
*/
public static function validate_url( $url ) {
$validation_url = add_query_arg(
self::VALIDATE_QUERY_VAR,
1,
array(
self::VALIDATE_QUERY_VAR => 1,
self::CACHE_BUST_QUERY_VAR => wp_rand(),
),
$url
);

$r = wp_remote_get( $validation_url, array(
'cookies' => wp_unslash( $_COOKIE ),
'sslverify' => false,
'headers' => array(
'Cache-Control' => 'no-cache',
),
) );
if ( is_wp_error( $r ) ) {
return $r;
}
if ( wp_remote_retrieve_response_code( $r ) >= 400 ) {
return new WP_Error(
wp_remote_retrieve_response_code( $r ),
wp_remote_retrieve_response_message( $r )
);
}
$json = wp_remote_retrieve_header( $r, self::VALIDATION_ERRORS_RESPONSE_HEADER_NAME );
if ( ! $json ) {
return new WP_Error( 'response_header_absent' );
Expand All @@ -1059,6 +1174,7 @@ public static function validate_url( $url ) {
if ( ! is_array( $validation_errors ) ) {
return new WP_Error( 'malformed_json_validation_errors' );
}

return $validation_errors;
}

Expand Down Expand Up @@ -1240,26 +1356,29 @@ public static function handle_bulk_action( $redirect, $action, $items ) {
if ( self::RECHECK_ACTION !== $action ) {
return $redirect;
}
$urls = array();
$remaining_invalid_urls = array();
foreach ( $items as $item ) {
$url = get_post_meta( $item, self::AMP_URL_META, true );
if ( ! empty( $url ) ) {
$urls[] = $url;
self::validate_url( $url );
if ( empty( $url ) ) {
continue;
}

$validation_errors = self::validate_url( $url );
if ( ! is_array( $validation_errors ) ) {
continue;
}

self::store_validation_errors( $validation_errors, $url );
if ( ! empty( $validation_errors ) ) {
$remaining_invalid_urls[] = $url;
}
}

// Get the URLs that still have errors after rechecking.
$args = array(
self::URLS_TESTED => count( $items ),
self::REMAINING_ERRORS => '0',
self::REMAINING_ERRORS => empty( $remaining_invalid_urls ) ? '0' : '1',
);
foreach ( $urls as $url ) {
if ( self::get_validation_status_post( $url ) ) {
$args[ self::REMAINING_ERRORS ] = '1';
break;
}
}

return add_query_arg( $args, $redirect );
}
Expand Down Expand Up @@ -1305,8 +1424,11 @@ public static function handle_inline_recheck( $post_id ) {
$url = wp_validate_redirect( wp_unslash( $_GET['recheck_url'] ) );
}
$validation_errors = self::validate_url( $url );
self::store_validation_errors( $validation_errors, $url );
$remaining_errors = ! empty( $validation_errors ) ? '1' : '0';
$remaining_errors = true;
if ( is_array( $validation_errors ) ) {
self::store_validation_errors( $validation_errors, $url );
$remaining_errors = ! empty( $validation_errors );
}

$redirect = wp_get_referer();
if ( ! $redirect || empty( $validation_errors ) ) {
Expand All @@ -1319,7 +1441,7 @@ public static function handle_inline_recheck( $post_id ) {
}
$args = array(
self::URLS_TESTED => '1',
self::REMAINING_ERRORS => $remaining_errors,
self::REMAINING_ERRORS => $remaining_errors ? '1' : '0',
);
wp_safe_redirect( add_query_arg( $args, $redirect ) );
exit();
Expand Down
Loading

0 comments on commit da5f4ad

Please sign in to comment.