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

Consider multisite for posts abstraction. #1004

Closed
wants to merge 4 commits into from
Closed

Consider multisite for posts abstraction. #1004

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
32b982d
Consider multisite for posts abstraction.
peterwilsoncc Jan 13, 2023
c82f6d5
Remove unreachable code.
peterwilsoncc Jan 17, 2023
60d805d
Expand coment explaining why methods are called.
peterwilsoncc Jan 17, 2023
5c93a48
What if we avoid statics.
peterwilsoncc Jan 24, 2023
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
117 changes: 109 additions & 8 deletions includes/classes/DistributorPost.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ class DistributorPost {
*/
private $source_site = [];

/**
* The site ID for the current site.
*
* @var int
*/
public $site_id;

/**
* Initialize the DistributorPost object.
*
Expand Down Expand Up @@ -176,6 +183,20 @@ public function __construct( $post ) {
$this->connection_direction = 'pulled';
$this->connection_id = (int) get_post_meta( $post->ID, 'dt_original_source_id', true );
}

$this->site_id = get_current_blog_id();

// Pre-populate the post data to account for switching sites.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you expound on what the problem is? I guess I'm not quite understanding why all the data below needs to be set when we are on a multisite. I'm assuming it has something to do with ensuring we have the right information from the right site but I guess not completely following without being able to see in what situations this comes into play

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ensuring we have the right information from the right site

This is the goal.

We we distribute network sites, we run switch_to_blog() fairly early on (I'll probably try to delay it) and calling the helper methods was returning either false or the wrong post's information if there was a post ID collision.

In the DistributorPost::post_data() method, we currently call get_the_title() and the ::get_media(), ::get_terms() and ::get_meta() methods.

I'm wanting to make sure they always return data related to the post and site we set up the object with.

I guess I need to improve the comment.

if ( ! is_multisite() ) {
return;
}

$this->get_permalink();
$this->get_post_thumbnail_id();
$this->get_post_thumbnail_url();
$this->get_the_post_thumbnail();
$this->get_meta();
$this->get_terms();
}

/**
Expand Down Expand Up @@ -314,7 +335,12 @@ public function get_the_id() {
* @return string Post permalink.
*/
public function get_permalink() {
return get_permalink( $this->post );
static $permalink = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not opposed to using statics here but does this actually help in practice? Are we calling these methods multiple times in the same execution?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We may... some of the canonical filters call get_permalink.

I'm not sure what's better or worse: making sure the site matches and switching if needs be or using statics. The former is slow but the latter may hit order of operations issues if other plugins add or remove filters.

if ( null === $permalink ) {
$permalink = get_permalink( $this->post );
}

return $permalink;
}

/**
Expand All @@ -323,7 +349,7 @@ public function get_permalink() {
* @return string Post type.
*/
public function get_post_type() {
return get_post_type( $this->post );
return $this->post->post_type;
}

/**
Expand All @@ -332,7 +358,12 @@ public function get_post_type() {
* @return int|false Post thumbnail ID or false if no thumbnail is set.
*/
public function get_post_thumbnail_id() {
return get_post_thumbnail_id( $this->post );
static $thumbnail_id = null;
if ( null === $thumbnail_id ) {
$thumbnail_id = get_post_thumbnail_id( $this->post );
}

return $thumbnail_id;
}

/**
Expand All @@ -342,7 +373,12 @@ public function get_post_thumbnail_id() {
* @return string|false The post's thumbnail URL or false if no thumbnail is set.
*/
public function get_post_thumbnail_url( $size = 'post-thumbnail' ) {
return get_the_post_thumbnail_url( $this->post, $size );
static $thumbnail_url = null;
if ( null === $thumbnail_url ) {
$thumbnail_url = get_the_post_thumbnail_url( $this->post, $size );
}

return $thumbnail_url;
}

/**
Expand All @@ -353,6 +389,23 @@ public function get_post_thumbnail_url( $size = 'post-thumbnail' ) {
* @return string|false The post's thumbnail HTML or false if no thumbnail is set.
*/
public function get_the_post_thumbnail( $size = 'post-thumbnail', $attr = '' ) {
static $thumbnail = [];
$hash = md5( wp_json_encode( [ $size, $attr ] ) );

if ( empty( $thumbnail[ $hash ] ) ) {
$switched = false;
if ( is_multisite() && get_current_blog_id() !== $this->site_id ) {
switch_to_blog( $this->site_id );
$switched = true;
}
$thumbnail[ $hash ] = get_the_post_thumbnail( $this->post, $size, $attr );

if ( $switched ) {
restore_current_blog();
}
}

return $thumbnail[ $hash ];
return get_the_post_thumbnail( $this->post, $size, $attr );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to remove this return statement

}

Expand Down Expand Up @@ -437,7 +490,21 @@ public function get_author_link( $author_link = '' ) {
|| ! $this->original_post_url
) {
if ( empty( $author_link ) ) {
return get_author_posts_url( $this->post->post_author );
static $author_posts_url = null;
if ( empty( $author_posts_url ) ) {
$switched = false;
if ( is_multisite() && get_current_blog_id() !== $this->site_id ) {
switch_to_blog( $this->site_id );
$switched = true;
}
$author_posts_url = get_author_posts_url( $this->post->post_author );

if ( $switched ) {
restore_current_blog();
}
}

return $author_posts_url;
}
return $author_link;
}
Expand All @@ -452,7 +519,12 @@ public function get_author_link( $author_link = '' ) {
* @return array Array of meta data.
*/
public function get_meta() {
return Utils\prepare_meta( $this->post->ID );
static $meta = null;
if ( null === $meta ) {
$meta = Utils\prepare_meta( $this->post->ID );
}

return $meta;
}

/**
Expand All @@ -463,7 +535,12 @@ public function get_meta() {
* }
*/
public function get_terms() {
return Utils\prepare_taxonomy_terms( $this->post->ID );
static $terms = null;
if ( null === $terms ) {
$terms = Utils\prepare_taxonomy_terms( $this->post->ID );
}

return $terms;
}

/**
Expand All @@ -472,6 +549,16 @@ public function get_terms() {
* @return array
*/
public function get_media() {
static $media_array = null;
if ( null !== $media_array ) {
return $media_array;
}

if ( is_multisite() && get_current_blog_id() !== $this->site_id ) {
switch_to_blog( $this->site_id );
$switched = true;
}

$post_id = $this->post->ID;
if ( $this->has_blocks() ) {
$raw_media = $this->parse_media_blocks();
Expand Down Expand Up @@ -500,6 +587,9 @@ public function get_media() {
$media_array[] = $featured_image;
}

if ( $switched ) {
restore_current_blog();
}
return $media_array;
}

Expand Down Expand Up @@ -615,7 +705,12 @@ private function parse_blocks_for_attachment_id( $block ) {
* }
*/
public function post_data() {
return [
if ( is_multisite() && get_current_blog_id() !== $this->site_id ) {
switch_to_blog( $this->site_id );
$switched = true;
}

$post_data = [
'title' => html_entity_decode( get_the_title( $this->post->ID ), ENT_QUOTES, get_bloginfo( 'charset' ) ),
'slug' => $this->post->post_name,
'post_type' => $this->post->post_type,
Expand All @@ -625,6 +720,12 @@ public function post_data() {
'distributor_terms' => $this->get_terms(),
'distributor_meta' => $this->get_meta(),
];

if ( $switched ) {
restore_current_blog();
}

return $post_data;
}

/**
Expand Down