Skip to content
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
15 changes: 13 additions & 2 deletions assets/src/components/JobStatus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type { JobStatusData } from '@/js/use-index-status';
// What each job is, and what its failure means for search and what to do.
const getJobCopy = (
job: JobStatusData
): { label: string; failed: string } | null => {
): { label?: string; failed: string } | null => {
switch ( job.job ) {
case 'index_site':
return {
Expand All @@ -21,6 +21,13 @@ const getJobCopy = (
'onesearch'
),
};
case 'delete_site_records':
return {
failed: __(
'Content from a removed site may still show in search. Re-index to try again.',
'onesearch'
),
Comment thread
justlevine marked this conversation as resolved.
};
default:
return null;
}
Expand All @@ -35,7 +42,7 @@ const getJobCopy = (
const JobStatus = ( { job }: { job: JobStatusData } ) => {
const copy = getJobCopy( job );

if ( ! copy || job.status === 'completed' ) {
if ( ! copy ) {
return null;
}

Expand All @@ -50,6 +57,10 @@ const JobStatus = ( { job }: { job: JobStatusData } ) => {
);
}

if ( job.status === 'completed' || ! copy.label ) {
return null;
}

const { progress } = job;

return (
Expand Down
6 changes: 5 additions & 1 deletion inc/Modules/Rest/Indexer_Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use OneSearch\Jobs\Queue;
use OneSearch\Modules\Search\Index_Status;
use OneSearch\Modules\Search\Indexer;
use OneSearch\Modules\Search\Jobs\Delete_Site_Records_Job;
use OneSearch\Modules\Search\Jobs\Index_Site_Job;
use OneSearch\Modules\Search\Jobs\Jobs;
use OneSearch\Modules\Settings\Settings;
Expand Down Expand Up @@ -71,12 +72,15 @@ public function get_status(): \WP_REST_Response {

/**
* Starts indexing all of the site's posts, and on a governing site all of each brand site's posts too. Indexing that's going starts over.
*
* A governing site also tries again to delete the records of the sites it removed, where that failed.
*/
public function reindex(): \WP_REST_Response|\WP_Error {
$sites = [ Utils::get_current_site_url() ];

if ( Settings::is_governing_site() ) {
$sites = [ ...$sites, ...array_keys( Settings::get_shared_sites() ) ];
Jobs::registry()->get( Delete_Site_Records_Job::class )->retry();
}

// An admin is watching.
Expand Down Expand Up @@ -108,7 +112,7 @@ private function get_report(): array {
];
$jobs = array_map(
static function ( array $reported ): array {
// Every job that reports a status keys its records by the site it's about.
// Every job that reports a status keys it by the site it shows on.
$status = [
'job' => $reported['job'],
'site' => $reported['key'],
Expand Down
3 changes: 1 addition & 2 deletions inc/Modules/Rest/Remote_Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,7 @@ public static function deregister_from_governing_site(): true|\WP_Error {
'brand-site',
$our_public_key,
[
'method' => \WP_REST_Server::DELETABLE,
'timeout' => 15, // phpcs:ignore WordPressVIPMinimum.Performance.RemoteRequestTimeout.timeout_timeout -- The governing site purges this site's Algolia records before responding.
'method' => \WP_REST_Server::DELETABLE,
]
);

Expand Down
80 changes: 80 additions & 0 deletions inc/Modules/Search/Jobs/Delete_Site_Records_Job.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php
/**
* The job that deletes the records of a site removed from the governing site.
*
* @package OneSearch\Modules\Search\Jobs
*/

declare(strict_types = 1);

namespace OneSearch\Modules\Search\Jobs;

use OneSearch\Jobs\Abstract_Job;
use OneSearch\Modules\Search\Indexer;
use OneSearch\Utils;

/**
* Class - Delete_Site_Records_Job
*/
final class Delete_Site_Records_Job extends Abstract_Job {
/**
* {@inheritDoc}
*
* A removed site isn't listed, so its failed deletes are reported as one failure of this site, with the last one's error. Only a failure is reported, since a delete that's waiting or done means nothing for search.
*/
public function get_statuses(): array {
$failed = array_filter( parent::get_statuses(), static fn ( array $status ): bool => self::STATUS_FAILED === $status['status'] );

return [] === $failed ? [] : [ [ 'key' => Utils::get_current_site_url() ] + end( $failed ) ];
Comment thread
justlevine marked this conversation as resolved.
}

/**
* Dispatches each delete that failed again.
*/
public function retry(): void {
foreach ( $this->records->get_records() as $site_url => $record ) {
if ( self::STATUS_FAILED === $record['status'] ) {
$this->dispatch( [ (string) $site_url ] );
}
}
}

/**
* Deletes the record of every delete, e.g. once the whole index is gone. One that's waiting then stops when it runs.
*/
public function clear(): void {
foreach ( array_keys( $this->records->get_records() ) as $site_url ) {
$this->records->delete_record( (string) $site_url );
}
}

/**
* {@inheritDoc}
*
* The key is the removed site's URL.
*
* @param array<int, mixed> $args The removed site's URL.
*/
protected function get_key( array $args ): string {
return (string) $args[0];
}

/**
* {@inheritDoc}
*
* @param array<int, mixed> $args The removed site's URL.
*
* @throws \RuntimeException When the records can't be deleted.
*/
protected function run( array $args ): bool {
$site_url = (string) $args[0];

$is_deleted = ( new Indexer() )->delete_site_records( [ $site_url ] );

if ( is_wp_error( $is_deleted ) ) {
throw new \RuntimeException( esc_html( Indexer::get_error_message( $is_deleted ) ) );
}

return true;
}
}
5 changes: 3 additions & 2 deletions inc/Modules/Search/Jobs/Jobs.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ final class Jobs implements Registrable {
* The jobs, by name.
*/
private const JOBS = [
'index_site' => Index_Site_Job::class,
'index_posts' => Index_Posts_Job::class,
'index_site' => Index_Site_Job::class,
'index_posts' => Index_Posts_Job::class,
'delete_site_records' => Delete_Site_Records_Job::class,
];

/**
Expand Down
32 changes: 17 additions & 15 deletions inc/Modules/Search/Settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use OneSearch\Contracts\Interfaces\Registrable;
use OneSearch\Encryptor;
use OneSearch\Modules\Rest\Remote_Client;
use OneSearch\Modules\Search\Jobs\Delete_Site_Records_Job;
use OneSearch\Modules\Search\Jobs\Jobs;
use OneSearch\Modules\Settings\Settings as Admin_Settings;
use OneSearch\Utils;
Expand Down Expand Up @@ -185,7 +186,7 @@ public function register_settings(): void {
}

/**
* Deletes the Algolia index when a governing site is changed to a consumer, along with its index status and its jobs about its brand sites.
* Deletes the Algolia index when a governing site is changed to a consumer, along with its index status and its jobs about its brand sites and the sites it removed.
*
* @internal Hook callback
*
Expand All @@ -206,34 +207,40 @@ public function on_site_type_change( $option, $old_value, $new_value ): void {
Jobs::registry()->delete_records( $site_url );
}

Jobs::registry()->get( Delete_Site_Records_Job::class )->clear();

( new Indexer() )->delete_index();
}

/**
* Deletes algolia entries and the jobs about a site when it's removed from the list of shared sites, and cleans it out of the search settings.
* Deletes the jobs about a site when it's removed from the list of shared sites, queues the job that deletes its Algolia records, and cleans it out of the search settings.
*
* A site that's added back forgets its jobs too, so a delete from its removal doesn't run or show.
*
* @internal Hook callback
*
* @param mixed $old_value The old value.
* @param mixed $new_value The new value.
*/
public function on_shared_sites_change( $old_value, $new_value ): void {
// If there is no old value, nothing to do.
if ( ! is_array( $old_value ) || empty( $old_value ) ) {
return;
}

$old_site_urls = array_map(
$old_site_urls = is_array( $old_value ) ? array_map(
static function ( $site ) {
return ! empty( $site['url'] ) ? Utils::normalize_url( $site['url'] ) : null;
},
$old_value
);
) : [];
$new_site_urls = is_array( $new_value ) ? array_map(
static function ( $site ) {
return ! empty( $site['url'] ) ? Utils::normalize_url( $site['url'] ) : null;
},
$new_value
) : [];

// A site that's added back forgets a failed delete of its records from when it was removed.
foreach ( array_filter( array_diff( $new_site_urls, $old_site_urls ) ) as $site_url ) {
Jobs::registry()->delete_records( $site_url );
}

$removed_sites = array_filter( array_diff( $old_site_urls, $new_site_urls ) );
if ( empty( $removed_sites ) ) {
return;
Expand All @@ -244,12 +251,7 @@ static function ( $site ) {
// Its jobs, like a failed re-index, no longer apply.
foreach ( $removed_sites as $site_url ) {
Jobs::registry()->delete_records( $site_url );
}

$is_deleted = ( new Indexer() )->delete_site_records( $removed_sites );

if ( is_wp_error( $is_deleted ) ) {
Utils::log( sprintf( 'Failed to delete the records of removed sites: %s', $is_deleted->get_error_message() ) );
Jobs::registry()->get( Delete_Site_Records_Job::class )->dispatch( [ $site_url ] );
}

// Remove from indexable entities.
Expand Down
27 changes: 27 additions & 0 deletions tests/js/IndexStatus.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,25 @@ describe( 'JobStatus', () => {
expect( screen.getByText( 'cURL error 7' ) ).toBeInTheDocument();
} );

it( "says what a failed delete of a removed site's content means for search", () => {
render(
<JobStatus
job={ job( {
job: 'delete_site_records',
status: 'failed',
error: 'Algolia is down.',
} ) }
/>
);

expect(
screen.getByText(
'Content from a removed site may still show in search. Re-index to try again.'
)
).toBeInTheDocument();
expect( screen.getByText( 'Algolia is down.' ) ).toBeInTheDocument();
} );

it( 'shows nothing for a completed job, or one it has no copy for', () => {
const { container, rerender } = render(
<JobStatus job={ job( { status: 'completed' } ) } />
Expand All @@ -70,6 +89,14 @@ describe( 'JobStatus', () => {

rerender( <JobStatus job={ job( { job: 'unknown' } ) } /> );
expect( container ).toBeEmptyDOMElement();

// A job that only reports its failures.
rerender(
<JobStatus
job={ job( { job: 'delete_site_records', status: 'running' } ) }
/>
);
expect( container ).toBeEmptyDOMElement();
} );
} );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

use OneSearch\Modules\Rest\Abstract_REST_Controller;
use OneSearch\Modules\Rest\Brand_Site_Controller;
use OneSearch\Modules\Search\Jobs\Jobs;
use OneSearch\Modules\Search\Settings as Search_Settings;
use OneSearch\Modules\Settings\Settings;
use OneSearch\Tests\TestCase;
Expand Down Expand Up @@ -104,12 +105,13 @@ public function test_remove_brand_site_removes_the_requesting_site_and_its_searc
]
);

$algolia_paths = [];
$algolia_requests = [];
$this->mock_algolia_http_client( $algolia_paths, null, null, $algolia_requests );
$this->fake_algolia_index();
$this->add_algolia_record( 'brand_one_post', 'https://brand-one.example.com/' );
$this->add_algolia_record( 'brand_two_post', 'https://brand-two.example.com/' );

// The shared sites listeners that purge a removed site's search data.
// The shared sites listeners that purge a removed site's search data, and the jobs that do it.
( new Search_Settings() )->register_hooks();
( new Jobs() )->register_hooks();

// Keep the cache purge for the remaining brand sites off the network.
$block_http = static fn () => new \WP_Error( 'http_request_blocked', 'Blocked in tests.' );
Expand All @@ -122,6 +124,7 @@ public function test_remove_brand_site_removes_the_requesting_site_and_its_searc
$request->set_header( 'X-OneSearch-Token', 'brand-one-key' );

$response = $this->server->dispatch( $request );
$this->run_jobs();

remove_filter( 'pre_http_request', $block_http );

Expand All @@ -133,15 +136,7 @@ public function test_remove_brand_site_removes_the_requesting_site_and_its_searc
Search_Settings::get_indexable_entities()['entities']
);

$delete_filters = [];
foreach ( $algolia_requests as $algolia_request ) {
if ( str_contains( $algolia_request['path'], '/deleteByQuery' ) ) {
$delete_filters[] = json_decode( $algolia_request['body'], true )['filters'] ?? '';
}
}
$this->assertCount( 1, $delete_filters );
$this->assertStringContainsString( 'site_url:"https://brand-one.example.com/"', $delete_filters[0] );
$this->assertStringNotContainsString( 'brand-two', $delete_filters[0] );
$this->assertSame( [ 'brand_two_post' ], array_keys( $this->algolia_records ) );
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use OneSearch\Modules\Rest\Indexer_Controller;
use OneSearch\Modules\Search\Jobs\Index_Site_Job;
use OneSearch\Modules\Search\Jobs\Jobs;
use OneSearch\Modules\Search\Settings as Search_Settings;
use OneSearch\Modules\Settings\Settings;
use OneSearch\Tests\TestCase;
use OneSearch\Utils;
Expand Down Expand Up @@ -234,6 +235,39 @@ public function test_reindex_indexes_the_posts_of_a_failed_run(): void {
$this->assertSame( [ $post_id ], $this->get_algolia_post_ids() );
}

/**
* A removed site's records that couldn't be deleted show on this site's row, and the next re-index deletes them.
*/
public function test_a_reindex_retries_a_failed_delete_of_a_removed_sites_records(): void {
( new Search_Settings() )->register_hooks();
$this->share_brand_sites( [ 'old' ] );
$this->add_algolia_record( 'old_post', 'https://old.example.com/' );

// Algolia is down when the brand site is removed.
$this->fake_algolia_index( '/deleteByQuery' );
Settings::remove_shared_site( 'https://old.example.com/' );
$this->run_jobs();

$this->assertSame(
[
[
'job' => 'delete_site_records',
'site' => Utils::get_current_site_url(),
'status' => 'failed',
'error' => 'Failed to delete Algolia records by given args. forced test exception',
],
],
$this->get_status()['jobs']
);

$this->fake_algolia_index();
$this->reindex();
$this->run_jobs();

$this->assertSame( [ 'index_site' ], array_column( $this->get_status()['jobs'], 'job' ) );
$this->assertSame( [], $this->algolia_records );
}

/**
* A re-index starts Action Scheduler's runner at the end of the request, so the run doesn't wait for WP-Cron.
*/
Expand Down
Loading
Loading