Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move validation background tasks behind feature flag filter until Site Scanning is implemented #5892

Merged
merged 3 commits into from
Feb 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/Validation/SavePostValidationEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use AmpProject\AmpWP\BackgroundTask\BackgroundTaskDeactivator;
use AmpProject\AmpWP\BackgroundTask\SingleScheduledBackgroundTask;
use AmpProject\AmpWP\DevTools\UserAccess;
use AmpProject\AmpWP\Infrastructure\Conditional;

/**
* SavePostValidationEvent class.
Expand All @@ -19,7 +20,7 @@
*
* @internal
*/
final class SavePostValidationEvent extends SingleScheduledBackgroundTask {
final class SavePostValidationEvent extends SingleScheduledBackgroundTask implements Conditional {

/**
* Instance of URLValidationProvider
Expand All @@ -42,6 +43,15 @@ final class SavePostValidationEvent extends SingleScheduledBackgroundTask {
*/
const BACKGROUND_TASK_NAME = 'amp_single_post_validate';

/**
* Check whether the service is currently needed.
*
* @return bool Whether needed.
*/
public static function is_needed() {
return URLValidationCron::is_needed();
}

/**
* Class constructor.
*
Expand Down Expand Up @@ -111,6 +121,7 @@ protected function should_schedule_event( $args ) {

$post = get_post( $id );

// @todo This needs to be limited to when the status is publish because otherwise the validation request will fail to be able to access the post, as the request is not authenticated.
if ( ! $post
||
wp_is_post_revision( $post )
Expand Down
30 changes: 29 additions & 1 deletion src/Validation/URLValidationCron.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use AmpProject\AmpWP\BackgroundTask\BackgroundTaskDeactivator;
use AmpProject\AmpWP\BackgroundTask\RecurringBackgroundTask;
use AmpProject\AmpWP\Infrastructure\Conditional;

/**
* URLValidationCron class.
Expand All @@ -18,7 +19,7 @@
*
* @internal
*/
final class URLValidationCron extends RecurringBackgroundTask {
final class URLValidationCron extends RecurringBackgroundTask implements Conditional {

/**
* ScannableURLProvider instance.
Expand Down Expand Up @@ -48,6 +49,33 @@ final class URLValidationCron extends RecurringBackgroundTask {
*/
const DEFAULT_SLEEP_TIME = 1;

/**
* Check whether the service is currently needed.
*
* @return bool Whether needed.
*/
public static function is_needed() {
/**
* Filters whether to enable URL validation cron tasks.
*
* This is a feature flag used to control whether the sample set of site URLs are scanned on a daily basis and
* whether post permalinks are scheduled for immediate validation as soon as they are updated by a user who has
* DevTools turned off. This conditional flag will be removed once Site Scanning is implemented, likely in v2.2.
*
* @link https://github.com/ampproject/amp-wp/issues/5750
* @link https://github.com/ampproject/amp-wp/issues/4779
* @link https://github.com/ampproject/amp-wp/issues/4795
* @link https://github.com/ampproject/amp-wp/issues/4719
* @link https://github.com/ampproject/amp-wp/issues/5671
* @link https://github.com/ampproject/amp-wp/issues/5101
* @link https://github.com/ampproject/amp-wp/issues?q=label%3A%22Site+Scanning%22
*
* @param bool $enabled Enabled.
* @internal
*/
return apply_filters( 'amp_temp_validation_cron_tasks_enabled', false );
}

/**
* Class constructor.
*
Expand Down
12 changes: 12 additions & 0 deletions tests/php/src/Validation/SavePostValidationEventTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use AmpProject\AmpWP\BackgroundTask\BackgroundTaskDeactivator;
use AmpProject\AmpWP\BackgroundTask\SingleScheduledBackgroundTask;
use AmpProject\AmpWP\DevTools\UserAccess;
use AmpProject\AmpWP\Infrastructure\Conditional;
use AmpProject\AmpWP\Infrastructure\Registerable;
use AmpProject\AmpWP\Infrastructure\Service;
use AmpProject\AmpWP\Tests\Helpers\AssertContainsCompatibility;
Expand Down Expand Up @@ -49,6 +50,17 @@ public function test__construct() {
$this->assertInstanceof( SavePostValidationEvent::class, $this->test_instance );
$this->assertInstanceof( Service::class, $this->test_instance );
$this->assertInstanceof( Registerable::class, $this->test_instance );
$this->assertInstanceof( Conditional::class, $this->test_instance );
}

/** @covers ::is_needed() */
public function test_is_needed() {
$this->assertFalse( SavePostValidationEvent::is_needed() );

add_filter( 'amp_temp_validation_cron_tasks_enabled', '__return_true' );
$this->assertTrue( SavePostValidationEvent::is_needed() );

remove_filter( 'amp_temp_validation_cron_tasks_enabled', '__return_true' );
}

/**
Expand Down
12 changes: 12 additions & 0 deletions tests/php/src/Validation/URLValidationCronTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use AmpProject\AmpWP\BackgroundTask\BackgroundTaskDeactivator;
use AmpProject\AmpWP\BackgroundTask\CronBasedBackgroundTask;
use AmpProject\AmpWP\Infrastructure\Conditional;
use AmpProject\AmpWP\Infrastructure\Registerable;
use AmpProject\AmpWP\Infrastructure\Service;
use AmpProject\AmpWP\Tests\Helpers\PrivateAccess;
Expand Down Expand Up @@ -45,13 +46,24 @@ public function test_register() {
$this->assertInstanceof( URLValidationCron::class, $this->test_instance );
$this->assertInstanceof( Service::class, $this->test_instance );
$this->assertInstanceof( Registerable::class, $this->test_instance );
$this->assertInstanceof( Conditional::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 ::is_needed() */
public function test_is_needed() {
$this->assertFalse( URLValidationCron::is_needed() );

add_filter( 'amp_temp_validation_cron_tasks_enabled', '__return_true' );
$this->assertTrue( URLValidationCron::is_needed() );

remove_filter( 'amp_temp_validation_cron_tasks_enabled', '__return_true' );
}

/** @covers ::schedule_event() */
public function test_schedule_event_with_no_user() {
$event_name = $this->call_private_method( $this->test_instance, 'get_event_name' );
Expand Down