Merge pull request #21442 from Yoast/21405-when-indexable-creation-is…

…-disabled-theres-redundant-db-queries-on-page-load-in-the-frontend-of-posts Add early return in the hierachy creation to make sure we don't run a useless query.
Yoast · Jun 27, 2024 · 5596f6e · 5596f6e
2 parents aadef3b + 65577c1
commit 5596f6e
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 26 deletions.
diff --git a/composer.json b/composer.json
@@ -91,7 +91,7 @@
  "Yoast\\WP\\SEO\\Composer\\Actions::check_coding_standards"
  ],
  "check-cs-thresholds": [
- "@putenv YOASTCS_THRESHOLD_ERRORS=2495",
+ "@putenv YOASTCS_THRESHOLD_ERRORS=2493",
  "@putenv YOASTCS_THRESHOLD_WARNINGS=253",
  "Yoast\\WP\\SEO\\Composer\\Actions::check_cs_thresholds"
  ],

diff --git a/src/builders/indexable-hierarchy-builder.php b/src/builders/indexable-hierarchy-builder.php
@@ -5,6 +5,7 @@
 use WP_Post;
 use WP_Term;
 use WPSEO_Meta;
+use Yoast\WP\SEO\Helpers\Indexable_Helper;
 use Yoast\WP\SEO\Helpers\Options_Helper;
 use Yoast\WP\SEO\Helpers\Post_Helper;
 use Yoast\WP\SEO\Models\Indexable;
@@ -20,9 +21,9 @@
 class Indexable_Hierarchy_Builder {
 
  /**
- * Holds a list of indexables where the ancestors are saved for.
+ * Holds a list of indexable ids where the ancestors are saved for.
  *
- * @var array
+ * @var array<int>
  */
  protected $saved_ancestors = [];
 
@@ -61,24 +62,34 @@ class Indexable_Hierarchy_Builder {
  */
  private $post;
 
+ /**
+ * Holds the Indexable_Helper instance.
+ *
+ * @var Indexable_Helper
+ */
+ private $indexable_helper;
+
  /**
  * Indexable_Author_Builder constructor.
  *
  * @param Indexable_Hierarchy_Repository $indexable_hierarchy_repository The indexable hierarchy repository.
  * @param Primary_Term_Repository $primary_term_repository The primary term repository.
  * @param Options_Helper $options The options helper.
  * @param Post_Helper $post The post helper.
+ * @param Indexable_Helper $indexable_helper The indexable helper.
  */
  public function __construct(
  Indexable_Hierarchy_Repository $indexable_hierarchy_repository,
  Primary_Term_Repository $primary_term_repository,
  Options_Helper $options,
- Post_Helper $post
+ Post_Helper $post,
+ Indexable_Helper $indexable_helper
  ) {
  $this->indexable_hierarchy_repository = $indexable_hierarchy_repository;
  $this->primary_term_repository = $primary_term_repository;
  $this->options = $options;
  $this->post = $post;
+ $this->indexable_helper = $indexable_helper;
  }
 
  /**
@@ -106,8 +117,11 @@ public function build( Indexable $indexable ) {
  return $indexable;
  }
 
- $this->indexable_hierarchy_repository->clear_ancestors( $indexable->id );
+ if ( ! $this->indexable_helper->should_index_indexable( $indexable ) ) {
+ return $indexable;
+ }
 
+ $this->indexable_hierarchy_repository->clear_ancestors( $indexable->id );
  $indexable_id = $this->get_indexable_id( $indexable );
  $ancestors = [];
  if ( $indexable->object_type === 'post' ) {
@@ -265,7 +279,7 @@ private function find_primary_term_id_for_post( $post ) {
  /**
  * Find the deepest term in an array of term objects.
  *
- * @param array $terms Terms set.
+ * @param array<WP_Term> $terms Terms set.
  *
  * @return int The deepest term ID.
  */

diff --git a/tests/Unit/Builders/Indexable_Hierarchy_Builder_Test.php b/tests/Unit/Builders/Indexable_Hierarchy_Builder_Test.php
@@ -6,6 +6,7 @@
 use Mockery;
 use WPSEO_Meta;
 use Yoast\WP\SEO\Builders\Indexable_Hierarchy_Builder;
+use Yoast\WP\SEO\Helpers\Indexable_Helper;
 use Yoast\WP\SEO\Helpers\Options_Helper;
 use Yoast\WP\SEO\Helpers\Post_Helper;
 use Yoast\WP\SEO\Repositories\Indexable_Hierarchy_Repository;
@@ -28,6 +29,13 @@
  */
 final class Indexable_Hierarchy_Builder_Test extends TestCase {
 
+ /**
+ * Holds the Indexable_Helper instance.
+ *
+ * @var Mockery\MockInterface|Indexable_Helper
+ */
+ private $indexable_helper;
+
  /**
  * Holds the Indexable_Hierarchy_Repository instance.
  *
@@ -83,12 +91,14 @@ protected function set_up() {
  $this->options = Mockery::mock( Options_Helper::class );
  $this->indexable_repository = Mockery::mock( Indexable_Repository::class );
  $this->post = Mockery::mock( Post_Helper::class );
+ $this->indexable_helper = Mockery::mock( Indexable_Helper::class );
 
  $this->instance = new Indexable_Hierarchy_Builder(
  $this->indexable_hierarchy_repository,
  $this->primary_term_repository,
  $this->options,
- $this->post
+ $this->post,
+ $this->indexable_helper
  );
  $this->instance->set_indexable_repository( $this->indexable_repository );
  }
@@ -112,7 +122,7 @@ public function test_no_parents() {
  $indexable->object_type = 'post';
  $indexable->object_id = 1;
  $indexable->has_ancestors = true;
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository->expects( 'clear_ancestors' )->with( 1 )->andReturnTrue();
  $this->options->expects( 'get' )->with( 'post_types-post-maintax' )->andReturn( '0' );
  $this->post->expects( 'get_post' )
@@ -145,7 +155,7 @@ public function test_no_parents() {
  public function test_no_parents_and_has_ancestors_set_to_false() {
  $indexable = $this->get_indexable( 1, 'post' );
  $indexable->has_ancestors = false;
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository
  ->expects( 'clear_ancestors' )
  ->once();
@@ -187,7 +197,7 @@ public function test_parents_not_set() {
  $indexable->object_type = 'post';
  $indexable->object_id = 1;
  $indexable->has_ancestors = true;
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository->expects( 'clear_ancestors' )->with( 1 )->andReturnTrue();
  $this->post->expects( 'get_post' )->with( 1 )->andReturn( (object) [ 'post_type' => 'post' ] );
  $this->indexable_hierarchy_repository->expects( 'add_ancestor' )->with( 1, 0, 0 );
@@ -229,7 +239,7 @@ public function test_post_parents() {
  $this->options->expects( 'get' )->with( 'post_types-post-maintax' )->andReturn( '0' );
 
  $this->indexable_repository->expects( 'find_by_id_and_type' )->with( 2, 'post' )->andReturn( $parent_indexable );
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->post->expects( 'get_post' )
  ->once()
  ->with( 1 )
@@ -271,7 +281,7 @@ public function test_post_parent_with_no_indexable_id_set() {
  $indexable = $this->get_indexable( 1, 'post' );
  $parent_indexable = $this->get_indexable( 0, 'post' );
  $parent_indexable->object_id = 2;
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository
  ->expects( 'clear_ancestors' )
  ->with( 1 )
@@ -334,7 +344,7 @@ public function test_post_parents_with_an_unindexed_ancestor() {
  $parent_indexable = $this->get_indexable( 2, 'post' );
 
  $parent_indexable->post_status = 'unindexed';
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository
  ->expects( 'clear_ancestors' )
  ->with( 1 )
@@ -389,7 +399,7 @@ public function test_post_parents_with_an_unindexed_ancestor() {
  public function test_post_parents_with_parent_already_added() {
  $indexable = $this->get_indexable( 1, 'post' );
  $parent_indexable = $this->get_indexable( 2, 'post' );
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository
  ->expects( 'clear_ancestors' )
  ->with( 1 )
@@ -446,7 +456,7 @@ public function test_post_parents_with_parent_already_added() {
  */
  public function test_post_parents_having_the_parent_is_the_main_object() {
  $indexable = $this->get_indexable( 1, 'post' );
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository
  ->expects( 'clear_ancestors' )
  ->with( 1 )
@@ -531,7 +541,7 @@ public function test_primary_term_parents() {
 
  $this->indexable_hierarchy_repository->expects( 'clear_ancestors' )->with( 1 )->andReturnTrue();
  $this->indexable_hierarchy_repository->expects( 'add_ancestor' )->with( 1, 2, 1 );
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->primary_term_repository->expects( 'find_by_post_id_and_taxonomy' )->with( 1, 'tag', false )->andReturn( $primary_term );
 
  $this->options->expects( 'get' )->with( 'post_types-post-maintax' )->andReturn( 'tag' );
@@ -576,7 +586,7 @@ public function test_primary_term_parents_and_term_is_unindexed() {
 
  $parent_indexable = $this->get_indexable( 2, 'term' );
  $parent_indexable->post_status = 'unindexed';
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  Monkey\Functions\expect( 'get_term' )
  ->once()
  ->with( 2 )
@@ -690,7 +700,7 @@ public function test_many_primary_term_parents() {
  'parent' => 0,
  ]
  );
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository->expects( 'clear_ancestors' )->with( 1 )->andReturnTrue();
  $this->indexable_hierarchy_repository->expects( 'add_ancestor' )->with( 1, 3, 2 );
  $this->indexable_hierarchy_repository->expects( 'add_ancestor' )->with( 1, 2, 1 );
@@ -747,7 +757,7 @@ public function test_term_parent() {
  $parent_indexable->object_type = 'term';
  $parent_indexable->object_id = 2;
  $parent_indexable->has_ancestors = true;
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  Monkey\Functions\expect( 'get_the_terms' )
  ->with( 1, 'tag' )
  ->andReturn(
@@ -825,7 +835,7 @@ public function test_term_parent_where_terms_not_array() {
  ]
  );
  Monkey\Functions\expect( 'get_post_meta' )->with( 1, WPSEO_Meta::$meta_prefix . 'primary_term', true )->andReturn( '' );
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository->expects( 'clear_ancestors' )->with( 1 )->andReturnTrue();
 
  $this->primary_term_repository->expects( 'find_by_post_id_and_taxonomy' )->with( 1, 'tag', false )->andReturnFalse();
@@ -868,7 +878,7 @@ public function test_term_parent_where_terms_empty() {
  $indexable->object_type = 'post';
  $indexable->object_id = 1;
  $indexable->has_ancestors = true;
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  Monkey\Functions\expect( 'get_the_terms' )->with( 1, 'tag' )->andReturn();
  Monkey\Functions\expect( 'get_term' )
  ->with( 2 )
@@ -978,7 +988,7 @@ public function test_deepest_term_parent() {
  ]
  );
  Monkey\Functions\expect( 'get_post_meta' )->with( 1, WPSEO_Meta::$meta_prefix . 'primary_term', true )->andReturn( '' );
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository->expects( 'clear_ancestors' )->with( 1 )->andReturnTrue();
  $this->indexable_hierarchy_repository->expects( 'add_ancestor' )->with( 1, 4, 2 );
  $this->indexable_hierarchy_repository->expects( 'add_ancestor' )->with( 1, 3, 1 );
@@ -1031,7 +1041,7 @@ public function test_term() {
  $parent_indexable->object_type = 'term';
  $parent_indexable->object_id = 2;
  $parent_indexable->has_ancestors = true;
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  Monkey\Functions\expect( 'get_term' )
  ->once()
  ->with( 1 )
@@ -1080,7 +1090,7 @@ public function test_term_with_ancestor_not_indexed() {
  $indexable = $this->get_indexable( 1, 'term' );
  $parent_indexable = $this->get_indexable( 2, 'term' );
  $parent_indexable->post_status = 'unindexed';
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  Monkey\Functions\expect( 'get_term' )
  ->once()
  ->with( 1 )
@@ -1135,7 +1145,7 @@ public function test_term_with_ancestor_not_indexed() {
  */
  public function test_term_with_ancestor_is_the_main_object() {
  $indexable = $this->get_indexable( 1, 'term' );
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  Monkey\Functions\expect( 'get_term' )
  ->once()
  ->with( 1 )
@@ -1192,6 +1202,7 @@ public function test_term_with_ancestor_is_already_added() {
  $indexable = $this->get_indexable( 1, 'term' );
  $parent_indexable = $this->get_indexable( 2, 'term' );
 
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  Monkey\Functions\expect( 'get_term' )
  ->once()
  ->with( 1 )
@@ -1279,7 +1290,7 @@ public function test_primary_term_parents_with_no_primary_term_set() {
  $parent_indexable->object_type = 'term';
  $parent_indexable->object_id = 3;
  $parent_indexable->has_ancestors = true;
-
+ $this->indexable_helper->expects( 'should_index_indexable' )->with( $indexable )->andReturnTrue();
  $this->indexable_hierarchy_repository
  ->expects( 'clear_ancestors' )
  ->with( 1 )