Skip to content

Commit 08504b6

Browse files
kraftbjkraftbj
authored
Sitemaps: use XMLWriter by default (#45527)
1 parent 5986a09 commit 08504b6

File tree

6 files changed

+12
-213
lines changed

6 files changed

+12
-213
lines changed

‎projects/plugins/jetpack/changelog/remove-jetpack-sitemaps-stickers‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: minor
2+
Type: enhancement
3+
4+
Sitemaps: use XMLWriter by default for more performant sitemap generation

‎projects/plugins/jetpack/modules/sitemaps/sitemap-buffer-factory.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,12 @@ public static function create( $type, $item_limit, $byte_limit, $time = '1970-01
3737
* Temporary filter to disallow XMLWriter usage.
3838
*
3939
* @since 14.7
40+
* @since $$next-version$$ Update default to true.
4041
* @module sitemaps
4142
*
42-
* @param bool $use_xmlwriter Whether to use XMLWriter. Current default is false.
43+
* @param bool $use_xmlwriter Whether to use XMLWriter. Current default is true.
4344
*/
44-
$use_xmlwriter = apply_filters( 'jetpack_sitemap_use_xmlwriter', false );
45+
$use_xmlwriter = apply_filters( 'jetpack_sitemap_use_xmlwriter', true );
4546

4647
// First try XMLWriter if available
4748
if ( $use_xmlwriter && class_exists( 'XMLWriter' ) ) {

‎projects/plugins/jetpack/tests/php/modules/sitemaps/Jetpack_Sitemap_Buffer_Factory_Test.php‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,6 @@
1616
class Jetpack_Sitemap_Buffer_Factory_Test extends WP_UnitTestCase {
1717
use \Automattic\Jetpack\PHPUnit\WP_UnitTestCase_Fix;
1818

19-
/** Test Setup */
20-
public function set_up() {
21-
parent::set_up();
22-
add_filter( 'jetpack_sitemap_use_xmlwriter', '__return_true' );
23-
}
24-
25-
public function tear_down() {
26-
parent::tear_down();
27-
remove_filter( 'jetpack_sitemap_use_xmlwriter', '__return_true' );
28-
}
29-
3019
/**
3120
* Test creating a page buffer with XMLWriter.
3221
*

‎projects/plugins/wpcomsh/changelog/remove-jetpack-sitemaps-stickers‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: patch
2+
Type: removed
3+
4+
JP Sitemaps: Removed sticker determination for XMLWriter

‎projects/plugins/wpcomsh/sitemaps/class-wpcomsh-sitemap-sticker-handlers.php‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class WPCOMSH_Sitemap_Sticker_Handlers {
1717
*/
1818
public static function init() {
1919
add_filter( 'jetpack_sitemap_suspend_cache_addition', array( __CLASS__, 'handle_cache_suspension' ), 10, 1 );
20-
add_filter( 'jetpack_sitemap_use_xmlwriter', array( __CLASS__, 'handle_xmlwriter_usage' ), 10, 1 );
20+
add_filter( 'jetpack_sitemap_use_xmlwriter', '__return_true', 10, 1 ); // Remove once Atomic has updated Jetpack.
2121
}
2222

2323
/**
@@ -30,16 +30,6 @@ public static function handle_cache_suspension( $suspend_cache_addition ) {
3030
return self::check_sticker_activation( 'jetpack-sitemaps-suspend-cache-addition', $suspend_cache_addition );
3131
}
3232

33-
/**
34-
* Handle XMLWriter usage based on stickers.
35-
*
36-
* @param mixed $use_xmlwriter Current XMLWriter usage state.
37-
* @return bool Whether to use XMLWriter.
38-
*/
39-
public static function handle_xmlwriter_usage( $use_xmlwriter ) {
40-
return self::check_sticker_activation( 'jetpack-sitemaps-use-xmlwriter', $use_xmlwriter );
41-
}
42-
4333
/**
4434
* Check if a blog sticker is active and should override the default behavior.
4535
*

‎projects/plugins/wpcomsh/tests/SitemapsStickerHandlersTest.php‎

Lines changed: 0 additions & 189 deletions
Original file line numberDiff line numberDiff line change
@@ -40,159 +40,6 @@ public function tear_down() {
4040
parent::tear_down();
4141
}
4242

43-
/**
44-
* Tests handle_xmlwriter_usage when has_blog_sticker returns true.
45-
*
46-
* @group jetpack-sitemap
47-
*/
48-
#[Group( 'jetpack-sitemap' )]
49-
public function test_handle_xmlwriter_usage_has_blog_sticker_returns_true() {
50-
$blog_id = 12345;
51-
52-
// Mock Jetpack_Options::get_option to return the blog ID
53-
add_filter(
54-
'jetpack_options',
55-
function ( $value, $option ) use ( $blog_id ) {
56-
if ( 'id' === $option ) {
57-
return $blog_id;
58-
}
59-
return $value;
60-
},
61-
10,
62-
2
63-
);
64-
65-
// Set up mock has_blog_sticker to return true
66-
$GLOBALS['test_has_blog_sticker_args'] = array(
67-
'sticker' => 'jetpack-sitemaps-use-xmlwriter',
68-
'blog_id' => $blog_id,
69-
);
70-
71-
// Test the method directly - no duplication!
72-
$result = WPCOMSH_Sitemap_Sticker_Handlers::handle_xmlwriter_usage( false );
73-
$this->assertTrue( $result );
74-
75-
$result = WPCOMSH_Sitemap_Sticker_Handlers::handle_xmlwriter_usage( true );
76-
$this->assertTrue( $result );
77-
78-
$result = WPCOMSH_Sitemap_Sticker_Handlers::handle_xmlwriter_usage( null );
79-
$this->assertTrue( $result );
80-
}
81-
82-
/**
83-
* Tests handle_xmlwriter_usage when has_blog_sticker exists but returns false.
84-
* Since wpcomsh_is_site_sticker_active is a real function that depends on Atomic_Persistent_Data
85-
* (which doesn't exist in test environment), it will return false, so the original value is preserved.
86-
*
87-
* @group jetpack-sitemap
88-
*/
89-
#[Group( 'jetpack-sitemap' )]
90-
public function test_handle_xmlwriter_usage_has_blog_sticker_false() {
91-
$blog_id = 12345;
92-
93-
// Mock Jetpack_Options::get_option to return the blog ID
94-
add_filter(
95-
'jetpack_options',
96-
function ( $value, $option ) use ( $blog_id ) {
97-
if ( 'id' === $option ) {
98-
return $blog_id;
99-
}
100-
return $value;
101-
},
102-
10,
103-
2
104-
);
105-
106-
// Set up mock has_blog_sticker to return false
107-
$GLOBALS['test_has_blog_sticker_return'] = false;
108-
109-
// Test the method directly - original value preserved
110-
$result = WPCOMSH_Sitemap_Sticker_Handlers::handle_xmlwriter_usage( false );
111-
$this->assertFalse( $result );
112-
113-
$result = WPCOMSH_Sitemap_Sticker_Handlers::handle_xmlwriter_usage( true );
114-
$this->assertTrue( $result );
115-
}
116-
117-
/**
118-
* Tests jetpack_sitemap_use_xmlwriter filter when both functions exist but both return false.
119-
* This test is now redundant with test_jetpack_sitemap_use_xmlwriter_has_blog_sticker_false_wpcomsh_real_function
120-
* since wpcomsh_is_site_sticker_active is a real function that will return false in test environment.
121-
*
122-
* Note: This test now verifies the same behavior as the previous test but with explicit documentation
123-
* that both functions return false in the test environment.
124-
*
125-
* @group jetpack-sitemap
126-
*/
127-
#[Group( 'jetpack-sitemap' )]
128-
public function test_jetpack_sitemap_use_xmlwriter_both_functions_return_false() {
129-
$blog_id = 12345;
130-
131-
// Mock Jetpack_Options::get_option to return the blog ID
132-
add_filter(
133-
'jetpack_options',
134-
function ( $value, $option ) use ( $blog_id ) {
135-
if ( 'id' === $option ) {
136-
return $blog_id;
137-
}
138-
return $value;
139-
},
140-
10,
141-
2
142-
);
143-
144-
// Set up mock has_blog_sticker to return false
145-
$GLOBALS['test_has_blog_sticker_return'] = false;
146-
147-
// wpcomsh_is_site_sticker_active will return false since \Atomic_Persistent_Data doesn't exist
148-
// No need to mock it since it's a real function
149-
150-
// Test with false input - should return false (original value)
151-
$result = apply_filters( 'jetpack_sitemap_use_xmlwriter', false );
152-
$this->assertFalse( $result );
153-
154-
// Test with true input - should return true (original value)
155-
$result = apply_filters( 'jetpack_sitemap_use_xmlwriter', true );
156-
$this->assertTrue( $result );
157-
158-
// Test with null input - should return null (original value)
159-
$result = apply_filters( 'jetpack_sitemap_use_xmlwriter', null );
160-
$this->assertNull( $result );
161-
162-
// Test with string input - should return string (original value)
163-
$result = apply_filters( 'jetpack_sitemap_use_xmlwriter', 'test' );
164-
$this->assertEquals( 'test', $result );
165-
}
166-
167-
/**
168-
* Tests jetpack_sitemap_use_xmlwriter filter when neither function exists.
169-
* This simulates environments where WordPress.com sticker functions are not available.
170-
*
171-
* @group jetpack-sitemap
172-
*/
173-
#[Group( 'jetpack-sitemap' )]
174-
public function test_jetpack_sitemap_use_xmlwriter_functions_do_not_exist() {
175-
// In normal environment, these functions won't exist
176-
// so the filter should return the original value
177-
178-
// Test with false input - should return false (original value)
179-
$result = apply_filters( 'jetpack_sitemap_use_xmlwriter', false );
180-
$this->assertFalse( $result );
181-
182-
// Test with true input - should return true (original value)
183-
$result = apply_filters( 'jetpack_sitemap_use_xmlwriter', true );
184-
$this->assertTrue( $result );
185-
186-
// Test with null input - should return null (original value)
187-
$result = apply_filters( 'jetpack_sitemap_use_xmlwriter', null );
188-
$this->assertNull( $result );
189-
190-
// Test with array input - should return array (original value)
191-
$test_array = array( 'test' => 'value' );
192-
$result = apply_filters( 'jetpack_sitemap_use_xmlwriter', $test_array );
193-
$this->assertEquals( $test_array, $result );
194-
}
195-
19643
/**
19744
* Tests jetpack_sitemap_suspend_cache_addition filter when has_blog_sticker function exists and returns true.
19845
*
@@ -343,40 +190,4 @@ public function test_jetpack_sitemap_suspend_cache_addition_functions_do_not_exi
343190
$result = apply_filters( 'jetpack_sitemap_suspend_cache_addition', $test_array );
344191
$this->assertEquals( $test_array, $result );
345192
}
346-
347-
/**
348-
* Tests jetpack_sitemap_use_xmlwriter filter when Jetpack_Options::get_option returns false.
349-
* This simulates error conditions where the blog ID cannot be retrieved.
350-
*
351-
* @group jetpack-sitemap
352-
*/
353-
#[Group( 'jetpack-sitemap' )]
354-
public function test_jetpack_sitemap_use_xmlwriter_jetpack_options_returns_false() {
355-
// Mock Jetpack_Options::get_option to return false
356-
add_filter(
357-
'jetpack_options',
358-
function ( $value, $option ) {
359-
if ( 'id' === $option ) {
360-
return false;
361-
}
362-
return $value;
363-
},
364-
10,
365-
2
366-
);
367-
368-
// Set up mock has_blog_sticker to return false (should not be called with false blog ID)
369-
$GLOBALS['test_has_blog_sticker_return'] = false;
370-
371-
// wpcomsh_is_site_sticker_active will return false since \Atomic_Persistent_Data doesn't exist
372-
// So the filter should return the original value
373-
374-
// Test with false input - should return false (original value)
375-
$result = apply_filters( 'jetpack_sitemap_use_xmlwriter', false );
376-
$this->assertFalse( $result );
377-
378-
// Test with true input - should return true (original value)
379-
$result = apply_filters( 'jetpack_sitemap_use_xmlwriter', true );
380-
$this->assertTrue( $result );
381-
}
382193
}

0 commit comments

Comments
 (0)