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

Jump to bottom

Sitemap: Make sitemap storing more efficient by not querying the full post content before saving #39572

Merged
merged 2 commits into from
Oct 2, 2024
Merged

Sitemap: Make sitemap storing more efficient by not querying the full post content before saving #39572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b48bf00
Make sitemap storing more efficient but not querying the full post co…
oskosk Sep 28, 2024
a98842e
changelog
oskosk Sep 28, 2024
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
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: enhancement

Make sitemap storing more efficient but not querying the full post content when storing the updated sitemap
30 changes: 27 additions & 3 deletions projects/plugins/jetpack/modules/sitemaps/sitemap-librarian.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ public function read_sitemap_data( $name, $type ) {
* If a sitemap with that type and name does not exist, create it.
* If a sitemap with that type and name does exist, update it.
*
* This method uses get_current_sitemap_post_id() for efficiency,
* as it only retrieves the post ID, which will be typically cached in the persistent object cache.
* This approach avoids loading unnecessary data (like post content) into memory,
* unlike using read_sitemap_data() which would retrieve the full post object.
*
* @access public
* @since 4.8.0
*
Expand All @@ -82,9 +87,9 @@ public function read_sitemap_data( $name, $type ) {
public function store_sitemap_data( $index, $type, $contents, $timestamp ) {
$name = jp_sitemap_filename( $type, $index );

$the_post = $this->read_sitemap_data( $name, $type );
$post_id = $this->get_current_sitemap_post_id( $name, $type );

if ( null === $the_post ) {
if ( null === $post_id ) {
// Post does not exist.
wp_insert_post(
array(
Expand All @@ -98,7 +103,7 @@ public function store_sitemap_data( $index, $type, $contents, $timestamp ) {
// Post does exist.
wp_insert_post(
array(
'ID' => $the_post['id'],
'ID' => $post_id,
'post_title' => $name,
'post_content' => base64_encode( $contents ), // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_encode
'post_type' => $type,
Expand All @@ -108,6 +113,25 @@ public function store_sitemap_data( $index, $type, $contents, $timestamp ) {
}
}

/**
* Get the current sitemap post ID.
*
* @param string $name The name of the sitemap.
* @param string $type The type of the sitemap.
* @return int|null The post ID if it exists, null otherwise.
*/
private function get_current_sitemap_post_id( $name, $type ) {
$args = array(
'post_type' => $type,
'post_status' => 'draft',
'posts_per_page' => 1,
'title' => $name,
'fields' => 'ids',
);

$query = new WP_Query( $args );
return $query->posts ? $query->posts[0] : null;
Copy link
Member

Choose a reason for hiding this comment

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

I have one question about this. Should we use the WP_Query for checking if it has posts instead, like $query->has_posts()?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is simple enough with the have_posts check. Looking at the source for have_posts, I don't see any benefits to use that than a simple check of the posts property.

}
/**
* Delete a sitemap by name and type.
*
Expand Down
Loading