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.

Sign up for GitHub

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

Jump to bottom

Search: Default state is no active index versions #4434

Closed
wants to merge 2 commits into from
+213 −138
Closed

Search: Default state is no active index versions #4434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3877d81
Search: Default state is no active index versions
rebeccahum May 3, 2023
1f783cc
Remove unused `$provide_default`
rebeccahum Jun 5, 2023
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
35 changes: 12 additions & 23 deletions search/includes/classes/class-versioning.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,43 +220,28 @@ public function get_inactive_versions( Indexable $indexable ) {
* Retrieve details about available index versions
*
* @param \ElasticPress\Indexable $indexable The Indexable for which to retrieve index versions
* @param bool $provide_default If on corrupted or incomplete versioning default version 1 should be provided
* @return array Array of index versions
*/
public function get_versions( Indexable $indexable, bool $provide_default = true ) {
public function get_versions( Indexable $indexable ) {
$versions = [];

if ( $indexable->global ) {
$versions = get_site_option( self::INDEX_VERSIONS_OPTION_GLOBAL, array() );
$versions = get_site_option( self::INDEX_VERSIONS_OPTION_GLOBAL );
} else {
$versions = get_option( self::INDEX_VERSIONS_OPTION, array() );
$versions = get_option( self::INDEX_VERSIONS_OPTION );
}

$slug = $indexable->slug;

if ( ! $this->versions_array_has_slug( $versions, $slug ) ) {
if ( $provide_default ) {
return array(
1 => array(
'number' => 1,
'active' => true,
'created_time' => null, // We don't know when it was actually created
'activated_time' => null,
),
);
} else {
return [];
}
if ( ! is_array( $versions ) || ! isset( $versions[ $indexable->slug ] ) ||
rinatkhaziev marked this conversation as resolved.
Show resolved Hide resolved
( isset( $versions[ $indexable->slug ] ) && ! is_array( $versions[ $indexable->slug ] ) ) ) {
return [];
}

// Normalize the versions to ensure consistency (have all fields, etc)
return array_map( array( $this, 'normalize_version' ), $versions[ $slug ] );
}

private function versions_array_has_slug( $versions, $slug ) {
return is_array( $versions ) && isset( $versions[ $slug ] ) && is_array( $versions[ $slug ] ) && ! empty( $versions[ $slug ] );
}

/**
* Normalize the fields of a version, to handle old or incomplete data
*
Expand Down Expand Up @@ -426,7 +411,11 @@ public function get_version( Indexable $indexable, $version_number ) {
public function add_version( Indexable $indexable ) {
$versions = $this->get_versions( $indexable );

$new_version_number = $this->get_next_version_number( $versions );
if ( empty( $versions ) ) {
$new_version_number = 1;
} else {
$new_version_number = $this->get_next_version_number( $versions );
}

if ( ! $new_version_number ) {
return new WP_Error( 'unable-to-get-next-version', 'Unable to determine next index version' );
Expand Down Expand Up @@ -948,7 +937,7 @@ public function maybe_self_heal() {

$indexables_to_heal = [];
foreach ( $indexables as $indexable ) {
$versions = $this->get_versions( $indexable, false );
$versions = $this->get_versions( $indexable );
if ( ! is_array( $versions ) || count( $versions ) === 0 ) {
$indexables_to_heal[] = $indexable;
}
Expand Down
104 changes: 74 additions & 30 deletions search/includes/classes/commands/class-corecommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,44 +125,88 @@ private function list_indexes() {
}

protected function maybe_setup_index_version( $assoc_args ) {
if ( array_key_exists( 'version', $assoc_args ) || array_key_exists( 'using-versions', $assoc_args ) ) {
$version_number = '';
$using_versions = $assoc_args['using-versions'] ?? false;
if ( $assoc_args['version'] ?? false ) {
$setup_flag = isset( $assoc_args['setup'] ) && $assoc_args['setup'];
$using_versions_flag = isset( $assoc_args['using-versions'] ) && $assoc_args['using-versions'];
$version_flag = isset( $assoc_args['version'] ) && $assoc_args['version'];

// For each indexable specified, override the version
if ( ! isset( $assoc_args['indexables'] ) ) {
$indexables = \ElasticPress\Indexables::factory()->get_all();
if ( empty( $indexables ) ) {
WP_CLI::Error( 'No indexables found!' );
}
} else {
$indexables = $this->parse_indexables( $assoc_args );
}

$search = \Automattic\VIP\Search\Search::instance();
foreach ( $indexables as $indexable ) {
if ( ! $version_flag && ! $using_versions_flag && ! $setup_flag ) {
// No flags set, so we need to get the active version to index.
$version_number = $search->versioning->get_active_version_number( $indexable );
if ( is_wp_error( $version_number ) ) {
WP_CLI::Error( sprintf( 'No active version found for %s', $indexable->slug ) );
}
$this->set_version( $indexable, $version_number );
continue;
}

if ( $version_flag ) {
// If version flag passed in, explicitly use that. This will also cover a --version && --setup case.
$version_number = $assoc_args['version'];
} elseif ( $using_versions ) {
$version_number = 'next';
$version = $search->versioning->get_version( $indexable, $version_number );
if ( ! $version ) {
WP_CLI::Error( sprintf( 'No version found for %d for %s', $version_number, $indexable->slug ) );
}

$this->set_version( $indexable, $version_number );
continue;
}

if ( $version_number ) {
$search = \Automattic\VIP\Search\Search::instance();

// For each indexable specified, override the version
$indexables = $this->parse_indexables( $assoc_args );

if ( $using_versions ) {
foreach ( $indexables as $indexable ) {
$current_versions = $search->versioning->get_versions( $indexable );
if ( count( $current_versions ) > 1 ) {
WP_CLI::error( sprintf(
'There needs to be only one version per indexable in order to automatically use versions to reindex. Please remove inactive versions for indexable "%s".',
$indexable->slug
) );
}
}
$current_versions = $search->versioning->get_versions( $indexable );
if ( $using_versions_flag ) {
if ( count( $current_versions ) > 1 ) {
WP_CLI::error( sprintf(
'There needs to be only one version per indexable in order to automatically use versions to reindex. Please remove inactive versions for indexable "%s".',
$indexable->slug
) );
}
if ( empty( $current_versions ) ) {
WP_CLI::error( sprintf(
'There needs to be at least one version in order to use the --using-versions parameter. Please add a version for indexable "%s".',
$indexable->slug
) );
}

foreach ( $indexables as $indexable ) {
$result = $search->versioning->add_version( $indexable );
if ( is_wp_error( $result ) ) {
WP_CLI::error( sprintf( 'Error adding new version: %s', $result->get_error_message() ) );
}
$version_number = $search->versioning->get_active_version_number( $indexable );
if ( is_wp_error( $version_number ) ) {
WP_CLI::Error( sprintf( 'No active version found for %s. Must have an active version to use the --using-versions parameter.', $indexable->slug ) );
}
$result = $search->versioning->add_version( $indexable );
if ( is_wp_error( $result ) ) {
WP_CLI::error( sprintf( 'Error adding new version: %s', $result->get_error_message() ) );
}
$version_number = 'next';
} else {
if ( empty( $current_versions ) ) {
// If no versions exist, create new version and activate it.
$result = $search->versioning->add_version( $indexable );
if ( is_wp_error( $result ) ) {
WP_CLI::error( sprintf( 'Error adding new version: %s', $result->get_error_message() ) );
}
$result = $search->versioning->activate_version( $indexable, 1 ); // Activate first version
if ( is_wp_error( $result ) ) {
WP_CLI::error( sprintf( 'Error activating version 1: %s', $result->get_error_message() ) );
}
}

foreach ( $indexables as $indexable ) {
$this->set_version( $indexable, $version_number );
// Check that there is an active version to use before assigning $version_number.
$active_version = $search->versioning->get_active_version_number( $indexable );
if ( is_wp_error( $active_version ) ) {
$result = $search->versioning->activate_version( $indexable, array_key_first( $current_versions ) ); // Activate first version
}
$version_number = 'active';
}
$this->set_version( $indexable, $version_number );
}
}

Expand Down
15 changes: 15 additions & 0 deletions tests/search/includes/classes/test-class-health.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,24 @@ class Health_Test extends WP_UnitTestCase {

public function tearDown(): void {
Constant_Mocker::clear();
delete_option( Versioning::INDEX_VERSIONS_OPTION );

parent::tearDown();
}

public function setUp(): void {
do_action( 'plugins_loaded' );

parent::setUp();

Constant_Mocker::define( 'VIP_ORIGIN_DATACENTER', 'foo' );

// Set-up initial version
$indexable = \ElasticPress\Indexables::factory()->get( 'post' );
self::$search_instance->versioning->add_version( $indexable );
self::$search_instance->versioning->activate_version( $indexable, 1 );
}

public static function setUpBeforeClass(): void {
self::$search_instance = new \Automattic\VIP\Search\Search();
self::$search_instance->init();
Expand Down
26 changes: 18 additions & 8 deletions tests/search/includes/classes/test-class-queue.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,6 @@ public static function setUpBeforeClass(): void {
}

require_once __DIR__ . '/../../../../search/search.php';

\Automattic\VIP\Search\Search::instance()->init();

// Required so that EP registers the Indexables
do_action( 'plugins_loaded' );

// Users indexable doesn't get registered by default, but we have tests that queue user objects
\ElasticPress\Indexables::factory()->register( new \ElasticPress\Indexable\User\User() );
}

public function setUp(): void {
Expand All @@ -49,6 +41,22 @@ public function setUp(): void {
$this->es = \Automattic\VIP\Search\Search::instance();
$this->es->init();

// Required so that EP registers the Indexables
do_action( 'plugins_loaded' );

// Users indexable doesn't get registered by default, but we have tests that queue user objects
\ElasticPress\Indexables::factory()->register( new \ElasticPress\Indexable\User\User() );

// Create user indexable and activate it
$user_indexable = \ElasticPress\Indexables::factory()->get( 'user' );
$this->es->versioning->add_version( $user_indexable );
$this->es->versioning->activate_version( $user_indexable, 1 );

// Create post indexable and activate it
$post_indexable = \ElasticPress\Indexables::factory()->get( 'post' );
$this->es->versioning->add_version( $post_indexable );
$this->es->versioning->activate_version( $post_indexable, 1 );

$this->queue = $this->es->queue;

$this->queue->schema->prepare_table();
Expand All @@ -61,6 +69,8 @@ public function setUp(): void {
public function tearDown(): void {
remove_filter( 'ep_do_intercept_request', [ $this, 'filter_index_exists_request_ok' ], PHP_INT_MAX );

delete_option( $this->es->versioning::INDEX_VERSIONS_OPTION );

parent::tearDown();
}

Expand Down
25 changes: 22 additions & 3 deletions tests/search/includes/classes/test-class-search.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,22 @@ public function setUp(): void {
}, E_USER_WARNING );
}

public function test_initial_state_no_versions() {
$this->init_es();

$indexable = \ElasticPress\Indexables::factory()->get( 'post' );
$versions = $this->search_instance->versioning->get_versions( $indexable );

$this->assertEquals( [], $versions, 'There should be no versions in the initial state' );

$skip = apply_filters( 'ep_skip_query_integration', false );
$this->assertTrue( $skip );
}

public function tearDown(): void {
restore_error_handler();

Constant_Mocker::clear();

parent::tearDown();
}

Expand All @@ -64,9 +76,12 @@ public function test_query_es_with_invalid_type() {
* Test `ep_index_name` filter for ElasticPress + VIP Search
*/
public function test__vip_search_filter_ep_index_name() {
Constant_Mocker::define( 'VIP_ORIGIN_DATACENTER', 'bar' );
$this->init_es();

$indexable = \ElasticPress\Indexables::factory()->get( 'post' );
$this->search_instance->versioning->add_version( $indexable );
$this->search_instance->versioning->activate_version( $indexable, 1 );

$index_name = apply_filters( 'ep_index_name', 'index-name', 1, $indexable );

Expand All @@ -79,9 +94,13 @@ public function test__vip_search_filter_ep_index_name() {
* On "global" indexes, such as users, no blog id will be present
*/
public function test__vip_search_filter_ep_index_name_global_index() {
Constant_Mocker::define( 'VIP_ORIGIN_DATACENTER', 'bar' );

$this->init_es();

$indexable = \ElasticPress\Indexables::factory()->get( 'post' );
$this->search_instance->versioning->add_version( $indexable );
$this->search_instance->versioning->activate_version( $indexable, 1 );

$index_name = apply_filters( 'ep_index_name', 'index-name', null, $indexable );

Expand Down Expand Up @@ -294,6 +313,8 @@ public function test__vip_search_filter_ep_index_name_with_overridden_version()
Constant_Mocker::define( 'FILES_CLIENT_SITE_ID', 123 );

$indexable = \ElasticPress\Indexables::factory()->get( 'post' );
$this->search_instance->versioning->add_version( $indexable );
$this->search_instance->versioning->activate_version( $indexable, 1 );

add_filter( 'ep_do_intercept_request', [ $this, 'filter_ok_es_requests' ], PHP_INT_MAX, 5 );

Expand All @@ -319,8 +340,6 @@ public function test__vip_search_filter_ep_index_name_with_overridden_version()
$index_name = apply_filters( 'ep_index_name', 'index-name', null, $indexable );

$this->assertEquals( 'vip-123-post', $index_name );

delete_option( Versioning::INDEX_VERSIONS_OPTION );
}

public function test__vip_search_filter__ep_global_alias() {
Expand Down
13 changes: 13 additions & 0 deletions tests/search/includes/classes/test-class-settingshealthjob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,22 @@ public static function tearDownAfterClass(): void {
public function setUp(): void {
parent::setUp();

Constant_Mocker::define( 'VIP_ORIGIN_DATACENTER', 'foo' );

\Automattic\VIP\Prometheus\Plugin::get_instance()->init_registry();
self::$search->load_collector();
\Automattic\VIP\Prometheus\Plugin::get_instance()->load_collectors();

$indexable = \ElasticPress\Indexables::factory()->get( 'post' );
self::$search->versioning->add_version( $indexable );
self::$search->versioning->activate_version( $indexable, 1 );
}

public function tearDown(): void {
parent::tearDown();

Constant_Mocker::clear();
delete_option( Versioning::INDEX_VERSIONS_OPTION );
}

public function test__process_indexables_settings_health_results__reports_error() {
Expand Down
Loading