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

Limit sync on term updates depending on posts number #3106

Merged
merged 11 commits into from
Nov 4, 2022
28 changes: 19 additions & 9 deletions includes/classes/IndexHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -465,15 +465,7 @@ protected function get_objects_to_index() {
*/
do_action( "ep_pre_{$this->args['method']}_index", $this->index_meta, ( $this->index_meta['start'] ? 'start' : false ), $indexable );

/**
* Filter number of items to index per cycle in the dashboard
*
* @since 2.1
* @hook ep_index_default_per_page
* @param {int} Entries per cycle
* @return {int} New number of entries
*/
$per_page = apply_filters( 'ep_index_default_per_page', Utils\get_option( 'ep_bulk_setting', 350 ) );
$per_page = $this->get_index_default_per_page();

if ( ! empty( $this->args['per_page'] ) ) {
$per_page = $this->args['per_page'];
Expand Down Expand Up @@ -1219,6 +1211,24 @@ protected function on_error_update_and_clean( $error ) {
);
}

/**
* Return the default number of documents to be sent to Elasticsearch on each batch.
*
* @since 4.4.0
* @return integer
*/
public function get_index_default_per_page() : int {
/**
* Filter number of items to index per cycle in the dashboard
*
* @since 2.1
* @hook ep_index_default_per_page
* @param {int} Entries per cycle
* @return {int} New number of entries
*/
return (int) apply_filters( 'ep_index_default_per_page', Utils\get_option( 'ep_bulk_setting', 350 ) );
}

/**
* Return singleton instance of class.
*
Expand Down
137 changes: 130 additions & 7 deletions includes/classes/Indexable/Post/SyncManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
use ElasticPress\Elasticsearch as Elasticsearch;
use ElasticPress\Indexables as Indexables;
use ElasticPress\SyncManager as SyncManagerAbstract;
use ElasticPress\Utils;
use ElasticPress\IndexHelper;

if ( ! defined( 'ABSPATH' ) ) {
// @codeCoverageIgnoreStart
Expand Down Expand Up @@ -61,13 +63,17 @@ public function setup() {
// Called just because we need to know somehow if $delete_all is set before action_queue_meta_sync() runs.
add_filter( 'delete_post_metadata', array( $this, 'maybe_delete_meta_for_all' ), 10, 5 );
add_action( 'deleted_post_meta', array( $this, 'action_queue_meta_sync' ), 10, 4 );
add_action( 'set_object_terms', array( $this, 'action_set_object_terms' ), 10, 6 );
add_action( 'edited_term', array( $this, 'action_edited_term' ), 10, 3 );
add_action( 'deleted_term_relationships', array( $this, 'action_deleted_term_relationships' ), 10, 3 );
add_action( 'wp_initialize_site', array( $this, 'action_create_blog_index' ) );

add_filter( 'ep_sync_insert_permissions_bypass', array( $this, 'filter_bypass_permission_checks_for_machines' ) );
add_filter( 'ep_sync_delete_permissions_bypass', array( $this, 'filter_bypass_permission_checks_for_machines' ) );

// Conditionally update posts associated with terms
add_action( 'ep_admin_notices', [ $this, 'maybe_display_notice_edit_single_term' ] );
add_action( 'ep_admin_notices', [ $this, 'maybe_display_notice_term_list_screen' ] );
add_action( 'set_object_terms', array( $this, 'action_set_object_terms' ), 10, 6 );
add_action( 'edited_term', array( $this, 'action_edited_term' ), 10, 3 );
add_action( 'deleted_term_relationships', array( $this, 'action_deleted_term_relationships' ), 10, 3 );
}

/**
Expand Down Expand Up @@ -359,6 +365,74 @@ public function action_sync_on_update( $post_id ) {
}
}

/**
* Depending on the number of posts associated with the term display an admin notice
*
* @since 4.4.0
* @param array $notices Current ElasticPress admin notices
* @return array
*/
public function maybe_display_notice_edit_single_term( $notices ) {
global $pagenow, $tag;

/**
* Make sure we're on a term-related page in the admin dashboard.
*/
if ( ! is_admin() || 'term.php' !== $pagenow || ! $tag instanceof \WP_Term ) {
return $notices;
}

if ( IndexHelper::factory()->get_index_default_per_page() >= $tag->count ) {
return $notices;
}

$notices['edited_single_term'] = [
'html' => sprintf(
/* translators: Sync Page URL */
__( 'Due to the number of posts associated with this term, you will need to <a href="%s">resync</a> after editing or deleting it.', 'elasticpress' ),
Utils\get_sync_url()
),
'type' => 'warning',
'dismiss' => true,
];

return $notices;
}

/**
* Depending on the number of posts display an admin notice in the Dashboard Terms List Screen
*
* @since 4.4.0
* @param array $notices Current ElasticPress admin notices
* @return array
*/
public function maybe_display_notice_term_list_screen( $notices ) {
global $pagenow, $tax;

/**
* Make sure we're on a term-related page in the admin dashboard.
*/
if ( ! is_admin() || 'edit-tags.php' !== $pagenow || ! $tax instanceof \WP_Taxonomy ) {
return $notices;
}

if ( ! $this->is_tax_max_count_bigger_than_items_per_cycle( $tax ) ) {
return $notices;
}

$notices['too_many_posts_on_term'] = [
'html' => sprintf(
/* translators: Sync Page URL */
__( 'Depending on the number of posts associated with a term, you may need to <a href="%s">resync</a> after editing or deleting it.', 'elasticpress' ),
Utils\get_sync_url()
),
'type' => 'warning',
'dismiss' => true,
];

return $notices;
}

/**
* When a post's terms are changed, re-index.
*
Expand Down Expand Up @@ -473,6 +547,9 @@ public function action_edited_term( $term_id, $tt_id, $taxonomy ) {
return;
}

// If we have more items to update than the number set as Content Items per Index Cycle, skip it.
$should_skip = count( $object_ids ) > IndexHelper::factory()->get_index_default_per_page();

/**
* Filter to allow skipping this action in case of custom handling
*
Expand All @@ -484,12 +561,10 @@ public function action_edited_term( $term_id, $tt_id, $taxonomy ) {
* @param {array} $object_ids IDs of the objects attached to the term id.
* @return {bool} New value of whether to skip running action_edited_term or not
*/
if ( apply_filters( 'ep_skip_action_edited_term', false, $term_id, $tt_id, $taxonomy, $object_ids ) ) {
if ( apply_filters( 'ep_skip_action_edited_term', $should_skip, $term_id, $tt_id, $taxonomy, $object_ids ) ) {
return;
}

$indexable = Indexables::factory()->get( $this->indexable_slug );

// Add all of them to the queue
foreach ( $object_ids as $post_id ) {
if ( ! $this->should_reindex_post( $post_id, $taxonomy ) ) {
Expand Down Expand Up @@ -652,6 +727,54 @@ protected function should_reindex_post( $post_id, $taxonomy ) {
return false;
}

return true;
// If we have more items to update than the number set as Content Items per Index Cycle, skip it to avoid a timeout.
$single_ids_queued = array_unique( array_keys( $this->sync_queue ) );
$has_too_many_queued = count( $single_ids_queued ) > IndexHelper::factory()->get_index_default_per_page();

return ! $has_too_many_queued;
}

/**
* Given a taxonomy, check if the term with most posts is under or above the number set as Content Items per Index Cycle.
*
* The result will be cached in a transient. Its TTL will depend on the result:
* If it is determined we have a term with more posts, cache it for more time.
*
* @since 4.4.0
* @param \WP_Taxonomy $tax The taxonomy object
* @return boolean
*/
protected function is_tax_max_count_bigger_than_items_per_cycle( \WP_Taxonomy $tax ) : bool {
$transient_name = "ep_term_max_count_{$tax->name}";
$cached_max_count = get_transient( $transient_name );

if ( is_integer( $cached_max_count ) ) {
return $cached_max_count > IndexHelper::factory()->get_index_default_per_page();
}

$max_count = get_terms(
[
'taxonomy' => $tax->name,
'orderby' => 'count',
'order' => 'DESC',
'number' => 1,
'count' => true,
]
);

if ( ! is_array( $max_count ) || ! $max_count[0] instanceof \WP_Term || ! is_integer( $max_count[0]->count ) ) {
set_transient( $transient_name, 0, HOUR_IN_SECONDS );
return false;
}

$is_max_count_bigger = $max_count[0]->count > IndexHelper::factory()->get_index_default_per_page();

set_transient(
$transient_name,
$max_count[0]->count,
$is_max_count_bigger ? DAY_IN_SECONDS : HOUR_IN_SECONDS
);

return $is_max_count_bigger;
}
}
17 changes: 17 additions & 0 deletions includes/utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -671,3 +671,20 @@ function get_asset_info( $slug, $attribute = null ) {

return $asset;
}

/**
* Return the Sync Page URL.
*
* @since 4.4.0
* @param boolean $do_sync Whether the link should or should not start a resync.
* @return string
*/
function get_sync_url( bool $do_sync = false ) : string {
$page = 'admin.php?page=elasticpress-sync';
if ( $do_sync ) {
$page .= '&do_sync';
}
return ( defined( 'EP_IS_NETWORK' ) && EP_IS_NETWORK ) ?
network_admin_url( $page ) :
admin_url( $page );
}
9 changes: 2 additions & 7 deletions tests/cypress/integration/dashboard-sync.cy.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
/* global indexNames */

describe('Dashboard Sync', () => {
function setPerIndexCycle(number = null) {
const newValue = number || 350;
cy.wpCli(`option set ep_bulk_setting ${newValue}`);
}

function canSeeIndexesNames() {
cy.visitAdminPage('admin.php?page=elasticpress-health');
cy.get('.metabox-holder')
Expand Down Expand Up @@ -145,7 +140,7 @@ describe('Dashboard Sync', () => {
});

it('Can pause the dashboard sync, can not activate a feature during sync nor perform a sync via WP-CLI', () => {
setPerIndexCycle(20);
cy.setPerIndexCycle(20);

cy.visitAdminPage('admin.php?page=elasticpress-sync');

Expand Down Expand Up @@ -176,6 +171,6 @@ describe('Dashboard Sync', () => {
cy.visitAdminPage('admin.php?page=elasticpress');
cy.get('.error-overlay').should('not.have.class', 'syncing');

setPerIndexCycle();
cy.setPerIndexCycle();
});
});
6 changes: 1 addition & 5 deletions tests/cypress/integration/features/comments.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,7 @@ describe('Comments Feature', () => {
cy.get('#submit').click();

// Check if the new comment was indexed
cy.wpCliEval(
`
$comments_index = \\ElasticPress\\Indexables::factory()->get( "comment" )->get_index_name();
WP_CLI::runcommand("elasticpress request {$comments_index}/_refresh --method=POST");`,
).then(() => {
cy.refreshIndex('comment').then(() => {
cy.wpCli('wp elasticpress stats')
.its('stdout')
.should('contain', `Documents: ${defaultApprovedComments + 1}`);
Expand Down
53 changes: 53 additions & 0 deletions tests/cypress/integration/indexables/post.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
describe('Post Indexable', () => {
it('Can conditionally update posts when a term is edited', () => {
/**
* At this point, using the default content:
* - the `Classic` (ID 29) term has 36 posts
* - the `Block` (ID 54) term has 7 posts
* Important: There is no post with both categories, as that would skew results.
*/
cy.setPerIndexCycle();
cy.visitAdminPage('edit-tags.php?taxonomy=category');
cy.get('div[data-ep-notice="too_many_posts_on_term"]').should('not.exist');

cy.setPerIndexCycle(35);
cy.visitAdminPage('edit-tags.php?taxonomy=category&orderby=count&order=desc');
cy.get('div[data-ep-notice="too_many_posts_on_term"]').should('exist');

// Change the `Classic` term, should not index
cy.visitAdminPage('term.php?taxonomy=category&tag_ID=29');
cy.get('div[data-ep-notice="edited_single_term"]').should('exist');
cy.get('#name').clearThenType('totallydifferenttermname');
cy.get('input.button-primary').click();

// Change the `Block` term, should index
cy.visitAdminPage('term.php?taxonomy=category&tag_ID=20');
cy.get('div[data-ep-notice="edited_single_term"]').should('not.exist');
cy.get('#name').clearThenType('b10ck');
cy.get('input.button-primary').click();

// Make sure the changes are processed by ES
cy.refreshIndex('post');

cy.visit('/?s=totallydifferenttermname');
cy.get('.hentry').should('not.exist');

cy.visit('/?s=b10ck');
cy.get('.hentry').should('exist');
cy.get('#debug-menu-target-EP_Debug_Bar_ElasticPress .ep-query-debug').should(
'contain.text',
'"name": "b10ck",',
);

// Restore
cy.visitAdminPage('term.php?taxonomy=category&tag_ID=29');
cy.get('#name').clearThenType('Classic');
cy.get('input.button-primary').click();

cy.visitAdminPage('term.php?taxonomy=category&tag_ID=20');
cy.get('#name').clearThenType('Block');
cy.get('input.button-primary').click();

cy.setPerIndexCycle();
});
});
12 changes: 12 additions & 0 deletions tests/cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -444,3 +444,15 @@ Cypress.Commands.add('createUser', (userData) => {
cy.get('#user_pass').clear().type(`${newUserDate.password}{enter}`);
}
});

Cypress.Commands.add('setPerIndexCycle', (number = 350) => {
cy.wpCli(`option set ep_bulk_setting ${number}`);
});

Cypress.Commands.add('refreshIndex', (indexable) => {
cy.wpCliEval(
`
$index = \\ElasticPress\\Indexables::factory()->get( "${indexable}" )->get_index_name();
WP_CLI::runcommand("elasticpress request {$index}/_refresh --method=POST");`,
);
});
Loading