Changeset 48532

Timestamp:

07/21/2020 01:55:45 PM (6 months ago)

Author:

swissspidy

Message:

Sitemaps: Correctly enforce maximum number of sitemaps in index.

Before this change, the limit of 50k entries was enforced for the number of providers, not the amount of sitemaps all providers add to the index in total.

Props pbiron, swissspidy.
Fixes #50666.

Location:

trunk

Files:

• src/wp-includes/sitemaps/class-wp-sitemaps-index.php (modified) (3 diffs)
• src/wp-includes/sitemaps/class-wp-sitemaps-registry.php (modified) (3 diffs)
• tests/phpunit/includes/bootstrap.php (modified) (1 diff)
• tests/phpunit/includes/class-wp-sitemaps-large-test-provider.php (added)
• tests/phpunit/tests/sitemaps/sitemaps-index.php (modified) (1 diff)

trunk/src/wp-includes/sitemaps/class-wp-sitemaps-index.php

@@ -26 +26 @@
 
     /**
+     * Maximum number of sitemaps to include in an index.
+     *
+     * @sincee 5.5.0
+     *
+     * @var int Maximum number of sitemaps.
+     */
+    private $max_sitemaps = 50000;
+
+    /**
      * WP_Sitemaps_Index constructor.
      *
@@ -48 +57 @@
         $providers = $this->registry->get_sitemaps();
         /* @var WP_Sitemaps_Provider $provider */
-        foreach ( $providers as $provider ) {
+        foreach ( $providers as $name => $provider ) {
             $sitemap_entries = $provider->get_sitemap_entries();
 
@@ -58 +67 @@
             // Using array_push is more efficient than array_merge in a loop.
             array_push( $sitemaps, ...$sitemap_entries );
+            if ( count( $sitemaps ) >= $this->max_sitemaps ) {
+                break;
+            }
         }
 
-        return $sitemaps;
+        return array_slice( $sitemaps, 0, $this->max_sitemaps, true );
     }
 

trunk/src/wp-includes/sitemaps/class-wp-sitemaps-registry.php

@@ -24 +24 @@
      */
     private $sitemaps = array();
-
-    /**
-     * Maximum number of sitemaps to include in an index.
-     *
-     * @sincee 5.5.0
-     *
-     * @var int Maximum number of sitemaps.
-     */
-    private $max_sitemaps = 50000;
 
     /**
@@ -70 +61 @@
 
     /**
-     * Lists all registered sitemaps.
+     * Returns all registered sitemap providers.
      *
      * @since 5.5.0
@@ -77 +68 @@
      */
     public function get_sitemaps() {
-        $total_sitemaps = count( $this->sitemaps );
-
-        if ( $total_sitemaps > $this->max_sitemaps ) {
-            return array_slice( $this->sitemaps, 0, $this->max_sitemaps, true );
-        }
-
         return $this->sitemaps;
     }

trunk/tests/phpunit/includes/bootstrap.php

@@ -163 +163 @@
 require __DIR__ . '/class-wp-sitemaps-test-provider.php';
 require __DIR__ . '/class-wp-sitemaps-empty-test-provider.php';
+require __DIR__ . '/class-wp-sitemaps-large-test-provider.php';
 
 /**

trunk/tests/phpunit/tests/sitemaps/sitemaps-index.php

@@ -19 +19 @@
         $sitemap_index = new WP_Sitemaps_Index( $registry );
         $this->assertCount( 24, $sitemap_index->get_sitemap_list() );
+    }
+
+    /**
+     * Test that a sitemap index won't contain more than 50000 sitemaps.
+     *
+     * @ticket 50666
+     */
+    public function test_get_sitemap_list_limit() {
+        $registry = new WP_Sitemaps_Registry();
+
+        // add 3 providers, which combined produce more than the maximum 50000 sitemaps in the index.
+        $registry->add_sitemap( 'provider_1', new WP_Sitemaps_Large_Test_Provider( 25000 ) );
+        $registry->add_sitemap( 'provider_2', new WP_Sitemaps_Large_Test_Provider( 25000 ) );
+        $registry->add_sitemap( 'provider_3', new WP_Sitemaps_Large_Test_Provider( 25000 ) );
+
+        $count = 0;
+        foreach ( $registry->get_sitemaps() as $provider ) {
+            $count += count( $provider->get_url_list( 1 ) );
+        }
+        $this->assertGreaterThan( 50000, $count );
+
+        $sitemap_index = new WP_Sitemaps_Index( $registry );
+        $this->assertCount( 50000, $sitemap_index->get_sitemap_list() );
     }