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

Fix: Term sync issue during the pagination #4102

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Fix: Term sync issue during the pagination #4102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
58f08a3
Fix: Term sync issue during the pagination
burhandodhy Mar 6, 2025
adf3668
Merge branch 'develop' into fix/term-indexable-issue
felipeelia Mar 6, 2025
f4574c7
Fix test
burhandodhy Mar 6, 2025
deb353f
Merge branch 'develop' into fix/term-indexable-issue
burhandodhy Mar 6, 2025
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
49 changes: 31 additions & 18 deletions includes/classes/Indexable/Term/Term.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,24 +251,8 @@ public function query_db( $args ) {
);
add_filter( 'terms_clauses', [ $this, 'bulk_indexing_filter_terms_where' ], 9999, 3 );

/**
* Filter database arguments for term count query
*
* @hook ep_term_all_query_db_args
* @param {array} $args Query arguments based to `wp_count_terms()`
* @since 3.4
* @return {array} New arguments
*/
$total_objects = wp_count_terms( apply_filters( 'ep_term_all_query_db_args', $all_query_args, $args ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we add this filter somewhere else now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filter is still used when advanced_pagination is not set

ElasticPress/includes/classes/Indexable/Term/Term.php

Line 268 in adf3668

$total_objects = wp_count_terms( apply_filters( 'ep_term_all_query_db_args', $all_query_args, $args ) );

$total_objects = ! is_wp_error( $total_objects ) ? (int) $total_objects : 0;

if ( ! empty( $args['offset'] ) ) {
if ( (int) $args['offset'] >= $total_objects ) {
$total_objects = 0;
}
}

$query = new WP_Term_Query( $args );
$query = new WP_Term_Query( $args );
$total_objects = $this->get_total_objects_for_query( $args );

remove_filter( 'terms_clauses', [ $this, 'bulk_indexing_filter_terms_where' ], 9999, 3 );
} else {
Expand Down Expand Up @@ -350,6 +334,35 @@ public function bulk_indexing_filter_terms_where( $clauses, $taxonomies, $args )
return $clauses;
}

/**
* Get the total number of terms for a given query.
*
* @param array $query_args The query args.
* @return int The total number of terms.
*/
protected function get_total_objects_for_query( $query_args ) {
static $object_counts = [];

// Reset the pagination-related args for optimal caching.
$normalized_query_args = array_merge(
$query_args,
[
'offset' => 0,
'paged' => 1,
'posts_per_page' => 1,
'no_found_rows' => false,
'ep_indexing_last_processed_object_id' => null,
]
);

$cache_key = md5( get_current_blog_id() . wp_json_encode( $normalized_query_args ) );
if ( ! isset( $object_counts[ $cache_key ] ) ) {
$object_counts[ $cache_key ] = wp_count_terms( $normalized_query_args );
}

return $object_counts[ $cache_key ];
}

/**
* Returns indexable taxonomies for the current site
*
Expand Down
92 changes: 79 additions & 13 deletions tests/php/indexables/TestTerm.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1483,7 +1483,7 @@ public function testQueryDb() {
);

$this->assertCount( 3, $results['objects'] );
$this->assertSame( 4, $results['total_objects'] );
$this->assertEquals( 4, $results['total_objects'] );

$results = $term_indexable->query_db(
[
Expand All @@ -1498,14 +1498,14 @@ public function testQueryDb() {
]
);

$this->assertSame( 0, $results['total_objects'] );
$this->assertEquals( 0, $results['total_objects'] );

// create new term
$term_1_id = $this->ep_factory->term->create();

// test only one term is returned
$results = $term_indexable->query_db( [ 'include' => $term_1_id ] );
$this->assertSame( 1, $results['total_objects'] );
$this->assertEquals( 1, $results['total_objects'] );

// test query returns all terms except one
$results = $term_indexable->query_db(
Expand All @@ -1514,7 +1514,7 @@ public function testQueryDb() {
'taxonomy' => 'post_tag',
]
);
$this->assertSame( 4, $results['total_objects'] );
$this->assertEquals( 4, $results['total_objects'] );

// create 5 new terms
$this->ep_factory->term->create_many( 2 );
Expand All @@ -1528,7 +1528,7 @@ public function testQueryDb() {
'taxonomy' => 'post_tag',
]
);
$this->assertSame( 5, $results['total_objects'] );
$this->assertEquals( 5, $results['total_objects'] );

// Test when lower limit is set and it returns only 3 terms.
$results = $term_indexable->query_db(
Expand All @@ -1537,7 +1537,7 @@ public function testQueryDb() {
'taxonomy' => 'post_tag',
]
);
$this->assertSame( 3, $results['total_objects'] );
$this->assertEquals( 3, $results['total_objects'] );

// Test when both upper and lower limit is set and it returns only 4 terms.
$results = $term_indexable->query_db(
Expand All @@ -1547,7 +1547,72 @@ public function testQueryDb() {
'taxonomy' => 'post_tag',
]
);
$this->assertSame( 4, $results['total_objects'] );
$this->assertEquals( 4, $results['total_objects'] );
}

/**
* Tests the pagination of the query_db method.
*
* @since 5.2.0
* @group term
*/
public function test_query_db_with_last_processed_object_id() {
$term_1_id = $this->ep_factory->term->create();
$term_2_id = $this->ep_factory->term->create();
$term_3_id = $this->ep_factory->term->create();

$term_indexable = new \ElasticPress\Indexable\Term\Term();

$results = $term_indexable->query_db(
[
'per_page' => 1,
'taxonomy' => 'post_tag',
]
);

$term_ids = wp_list_pluck( $results['objects'], 'ID' );
$this->assertEquals( $term_3_id, $term_ids[0] );
$this->assertCount( 1, $results['objects'] );
$this->assertEquals( 3, $results['total_objects'] );

// Second loop.
$results = $term_indexable->query_db(
[
'per_page' => 1,
'taxonomy' => 'post_tag',
'ep_indexing_last_processed_object_id' => $term_3_id,
]
);

$term_ids = wp_list_pluck( $results['objects'], 'ID' );
$this->assertEquals( $term_2_id, $term_ids[0] );
$this->assertCount( 1, $results['objects'] );
$this->assertEquals( 3, $results['total_objects'] );
}

/**
* Tests that the query_db method returns results sorted by ID.
*
* @since 5.2.0
* @group term
*/
public function test_query_db_sort_by() {
$term_1_id = $this->ep_factory->term->create();
$term_2_id = $this->ep_factory->term->create();
$term_3_id = $this->ep_factory->term->create();

$term_indexable = new \ElasticPress\Indexable\Term\Term();
$results = $term_indexable->query_db(
[
'taxonomy' => 'post_tag',
]
);

$term_ids = wp_list_pluck( $results['objects'], 'ID' );
$this->assertEquals( 3, $results['total_objects'] );
$this->assertEquals( $term_3_id, $term_ids[0] );
$this->assertEquals( $term_2_id, $term_ids[1] );
$this->assertEquals( $term_1_id, $term_ids[2] );
}

/**
Expand Down Expand Up @@ -1576,17 +1641,18 @@ function ( $clauses ) {

$results = $term_indexable->query_db(
[
'taxonomy' => 'post_tag',
'taxonomy' => 'post_tag',
'cache_buster' => wp_generate_uuid4(), // get_total_objects_for_query returns a cached value because test_query_db_sort_by calls query_db with the same query args.
]
);

$this->assertSame( 4, $results['total_objects'] );
$this->assertEquals( 4, $results['total_objects'] );

$term_ids = wp_list_pluck( $results['objects'], 'ID' );
$this->assertSame( $term_4_id, $term_ids[0] );
$this->assertSame( $term_3_id, $term_ids[1] );
$this->assertSame( $term_2_id, $term_ids[2] );
$this->assertSame( $term_1_id, $term_ids[3] );
$this->assertEquals( $term_4_id, $term_ids[0] );
$this->assertEquals( $term_3_id, $term_ids[1] );
$this->assertEquals( $term_2_id, $term_ids[2] );
$this->assertEquals( $term_1_id, $term_ids[3] );
}

/**
Expand Down